Chat UX Enhancements#3039
Merged
Merged
Conversation
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:
📝 WalkthroughWalkthroughAdds interactive question-overlay support, replaces the plain-text composer with a Tiptap-based prompt editor (slash commands and file mentions), consolidates tool-call rendering into a new ToolCallRow pattern, forwards extracted custom skills as preload metadata to the runtime, and wires PendingQuestion lifecycle events end-to-end. Changes
Sequence DiagramsequenceDiagram
participant User
participant ChatUI
participant TiptapEditor
participant Backend
participant Agent
User->>TiptapEditor: Type message (may include /cmd or `@file`)
TiptapEditor->>ChatUI: Show suggestions / preview
User->>ChatUI: Submit message
ChatUI->>Backend: sendMessage(payload + metadata.skills?)
Backend->>Agent: Forward with preloadSkills (if present)
Agent->>Backend: emit ask_question event
Backend->>ChatUI: lifecycle PendingQuestion
ChatUI->>ChatUI: set pendingQuestion + show QuestionInputOverlay
User->>ChatUI: choose option or submit custom answer
ChatUI->>Backend: respondToQuestion RPC
Backend->>Agent: deliver answer
Agent->>Backend: continue/complete response
Backend->>ChatUI: tool_result / updated message
ChatUI->>ChatUI: clear pendingQuestion, scroll to bottom
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
michalkopanski
force-pushed
the
chat-ux-enhancements
branch
from
a4fe11e to
618b9ac
Compare
Greptile SummaryThis PR is a comprehensive overhaul of the chat UI/UX: it introduces a unified Key changes and findings:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant ChatInputFooter
participant QuestionInputOverlay
participant ChatMessageList
participant Runtime
participant NotificationManager
User->>ChatInputFooter: sends message
ChatInputFooter->>Runtime: sendMessage()
Runtime-->>ChatMessageList: stream tool_call (ask_user)
Runtime-->>NotificationManager: PendingQuestion lifecycle event
NotificationManager-->>User: OS toast "Awaiting Response"
Note over ChatInputFooter: pendingQuestion set → overlay renders
ChatMessageList->>ChatMessageList: ScrollAnchor scrolls to bottom
User->>QuestionInputOverlay: selects option / types answer
QuestionInputOverlay->>Runtime: onRespond(questionId, answer)
Note over ChatInputFooter: optimistically hides overlay
Note over ChatInputFooter: textInput.focus() via rAF
Runtime-->>ChatMessageList: stream tool_result (answer)
ChatMessageList->>ChatMessageList: ScrollAnchor re-pins after setTimeout(10ms)
Runtime-->>NotificationManager: Stop lifecycle event
Note over NotificationManager: pane status → idle (was permission)
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
michalkopanski
force-pushed
the
chat-ux-enhancements
branch
from
063715b to
641e44a
Compare
![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: 14
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/chat/src/server/trpc/zod.ts (1)
85-92:⚠️ Potential issue | 🟠 MajorKeep
skillsmetadata consistent with restart/edit flows.This new field is accepted on
sendMessageInput, but the parallelrestartFromMessageInputpath still only carriesmodelandthinkingLevel. Editing or replaying a message will therefore drop the selected skills and can produce a different run than the original.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/chat/src/server/trpc/zod.ts` around lines 85 - 92, The metadata schema used for sendMessageInput includes a skills field but the parallel restartFromMessageInput/edit/replay path still omits it, causing skills to be dropped; update the metadata Zod object used by restartFromMessageInput (the same object that currently references model and thinkingLevel/ thinkingLevelSchema) to include skills: z.array(z.string()).optional() so the selected skills are preserved across restart/edit/replay flows and align with sendMessageInput.
🟡 Minor comments (14)
packages/ui/src/components/ai-elements/conversation.tsx-27-49 (1)
27-49:⚠️ Potential issue | 🟡 MinorProp spread can override internal
onMouseDownhandler.At lines 46–48,
onMouseDown={handleMouseDown}is followed by{...props}, allowing a consumer-providedonMouseDownto replace your handler and preventstopScroll()from executing.While no current callers pass
onMouseDown, the composition issue should be fixed to guarantee the internal behavior regardless of consumer props. ExtractonMouseDownfrom destructuring and compose handlers:Proposed fix
export const ConversationContent = ({ className, + onMouseDown, ...props }: ConversationContentProps) => { const { stopScroll } = useStickToBottomContext(); const handleMouseDown = useCallback( (e: React.MouseEvent) => { if ((e.target as Element).closest("[data-tool-trigger]")) { // Unpin from bottom so the resize handler never jumps the scroll position. stopScroll(); } + onMouseDown?.(e); }, - [stopScroll], + [onMouseDown, stopScroll], ); return ( <StickToBottom.Content className={cn("flex flex-col gap-8 p-4 select-text", className)} scrollClassName="[overflow-anchor:none]" onMouseDown={handleMouseDown} {...props} /> ); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ai-elements/conversation.tsx` around lines 27 - 49, The ConversationContent component currently spreads {...props} after setting onMouseDown, allowing a consumer onMouseDown to override handleMouseDown; extract onMouseDown from the incoming props in the function signature (e.g., const { onMouseDown: userOnMouseDown, ...rest } = props or via param destructuring), create a composed handler that calls handleMouseDown(e) first (or always) and then calls userOnMouseDown?.(e), and pass that composed handler to StickToBottom.Content while spreading the remaining props (rest) so internal stopScroll invoked by handleMouseDown cannot be overridden.packages/ui/src/components/ui/input-group.tsx-80-81 (1)
80-81:⚠️ Potential issue | 🟡 MinorExpand focus targeting to cover
textareaand avoid false matches oncontenteditable="false".On line 80, the selector
"input"skipsInputGroupTextareaelements when used with an addon. On line 81, the"[contenteditable]"selector incorrectly matches elements withcontenteditable="false".Proposed fix
- const focusTarget = - parent?.querySelector<HTMLElement>("input") ?? - parent?.querySelector<HTMLElement>("[contenteditable]"); + const focusTarget = + parent?.querySelector<HTMLElement>( + "[data-slot=input-group-control]:is(input,textarea)", + ) ?? + parent?.querySelector<HTMLElement>( + "[contenteditable]:not([contenteditable='false'])", + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ui/input-group.tsx` around lines 80 - 81, The current focus selector uses parent?.querySelector<HTMLElement>("input") and parent?.querySelector<HTMLElement>("[contenteditable]"), which skips <textarea> and incorrectly matches contenteditable="false"; update the queries to include textarea and exclude false contenteditable values — replace the first selector with one that targets both inputs and textareas (e.g., "input, textarea") and replace the contenteditable selector with one that only matches editable elements (e.g., '[contenteditable]:not([contenteditable="false"])'), updating the code paths where these parent?.querySelector<HTMLElement>(...) calls appear in the InputGroup component so focus behaves correctly for InputGroupTextarea and contenteditable elements.apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/SubagentToolCall/utils/parseSubagentToolResult/parseSubagentToolResult.ts-35-37 (1)
35-37:⚠️ Potential issue | 🟡 MinorDetailed tool-call parsing misses start-of-content blocks.
Line 35 requires a leading newline before
<subagent-tool-calls>. If the block is at offset 0, detailed parsing is skipped and structured tool data is lost.Proposed fix
- /\n<subagent-tool-calls>([\s\S]*?)<\/subagent-tool-calls>/, + /\n?<subagent-tool-calls>([\s\S]*?)<\/subagent-tool-calls>/,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/SubagentToolCall/utils/parseSubagentToolResult/parseSubagentToolResult.ts` around lines 35 - 37, The regex in parseSubagentToolResult requires a leading newline before the <subagent-tool-calls> tag so blocks at offset 0 are ignored; update the regex used to find the subagent tool block (the pattern around "<subagent-tool-calls>([\s\S]*?)</subagent-tool-calls>") to not require a leading "\n" (e.g., remove the explicit "\n" or make it optional) so match is found when the tag is at the start of the content and the existing match handling can extract the structured tool data as intended.apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/ChatMessageList/components/SubagentExecutionMessage/utils/toSubagentViewModels.ts-93-96 (1)
93-96:⚠️ Potential issue | 🟡 MinorUse
asRecord()here so arrays don't leak through as tool args.
typeof record.args === "object"also matches arrays, butSubagentToolCall["args"]is declared asRecord<string, unknown> | null. If a tool emits positional args, downstream code gets an array masquerading as an object.♻️ Proposed fix
- args: - typeof record.args === "object" && record.args !== null - ? (record.args as Record<string, unknown>) - : null, + args: asRecord(record.args),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/ChatMessageList/components/SubagentExecutionMessage/utils/toSubagentViewModels.ts around lines 93 - 96, The current check in toSubagentViewModels uses typeof record.args === "object" which also matches arrays and allows positional args arrays to leak into SubagentToolCall["args"]; replace that logic with a call to asRecord(record.args) (or equivalent helper) so only plain object maps are accepted and arrays become null, and add/import asRecord where needed; ensure the resulting value assigned to args is either the Record<string, unknown> returned by asRecord or null.packages/ui/src/components/ai-elements/web-fetch-tool.tsx-46-69 (1)
46-69:⚠️ Potential issue | 🟡 MinorTreat all 2xx responses as success, and keep the row error state in sync.
Right now anything other than
200is rendered as an error, so valid204/206fetches will show destructive status.ToolCallRowalso stays non-error for those cases becauseisErroronly tracks"output-error".♻️ Proposed fix
- const isSuccess = statusCode === 200; + const isHttpError = + statusCode !== undefined && (statusCode < 200 || statusCode >= 300); const hasContent = Boolean(content); const hostname = url ? extractHostname(url) : ""; const statusNode = isPending ? ( <div className="flex h-6 w-6 items-center justify-center"> <Loader size={12} /> </div> - ) : isError || !isSuccess ? ( + ) : isError || isHttpError ? ( <span className="text-xs text-destructive"> {statusCode ? `Error ${statusCode}` : "Failed"} </span> ) : bytes !== undefined ? ( <span className="text-xs text-muted-foreground">{formatBytes(bytes)}</span> @@ - isError={isError} + isError={isError || isHttpError}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ai-elements/web-fetch-tool.tsx` around lines 46 - 69, The component currently treats only 200 as success; update the success check to treat any 2xx status as success by changing isSuccess to statusCode >= 200 && statusCode < 300 (or equivalent), then make the ToolCallRow error state reflect both explicit output errors and non-success HTTP responses by passing isError || (!isPending && !isSuccess) (or compute a new rowIsError variable) into the ToolCallRow isError prop and use that same success flag when choosing the statusNode so 204/206 are shown as successful rather than destructive.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPaneInterface/components/ChatMessageList/components/SubagentExecutionMessage/utils/toSubagentViewModels.ts-93-96 (1)
93-96:⚠️ Potential issue | 🟡 MinorUse
asRecord()here so arrays don't leak through as tool args.
typeof record.args === "object"also matches arrays, butSubagentToolCall["args"]is declared asRecord<string, unknown> | null. If a tool emits positional args, downstream code gets an array masquerading as an object.♻️ Proposed fix
- args: - typeof record.args === "object" && record.args !== null - ? (record.args as Record<string, unknown>) - : null, + args: asRecord(record.args),🤖 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/ChatPane/ChatPaneInterface/components/ChatMessageList/components/SubagentExecutionMessage/utils/toSubagentViewModels.ts` around lines 93 - 96, Replace the current typeof-based args check in toSubagentViewModels (the block assigning args: ... from record.args) with a call to asRecord(record.args) so arrays will not be treated as objects; ensure the assigned type matches SubagentToolCall["args"] (Record<string, unknown> | null) by using asRecord(record.args) which returns a Record or null.apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/FileMentionNode.tsx-9-11 (1)
9-11:⚠️ Potential issue | 🟡 MinorGuard against null/undefined
pathattribute.The
pathattribute has a default ofnull, but the component assumes it's a string when calling.split("/"). If the node is created without a path, this will throw.🛡️ Proposed defensive fix
function FileMentionChip({ node, selected }: NodeViewProps) { - const path = node.attrs.path as string; + const path = (node.attrs.path as string | null) ?? ""; const name = path.split("/").pop() ?? path;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/FileMentionNode.tsx` around lines 9 - 11, FileMentionChip assumes node.attrs.path is a string and calls .split("/"), which can throw when path is null/undefined; update FileMentionChip to defensively read node.attrs.path (e.g., check typeof node.attrs.path === "string" or use a fallback string) before splitting and derive name from that safe value (provide a sensible fallback like empty string or "unknown") so FileMentionChip and its use of node.attrs.path are null-safe.apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/ChatMessageList/components/SubagentExecutionMessage/SubagentExecutionMessage.tsx-83-89 (1)
83-89:⚠️ Potential issue | 🟡 MinorDon't hardcode Mermaid to the light theme here.
This renders subagent markdown with
"default"even in dark mode, so diagrams can be inconsistent or hard to read compared with the main message renderer. Thread the current theme through this component the same wayMessagePartsRendererdoes.🎨 Suggested fix
import { Message, MessageContent, MessageResponse, } from "@superset/ui/ai-elements/message"; import { cn } from "@superset/ui/lib/utils"; import { SubagentInnerToolCall } from "renderer/components/Chat/components/SubagentInnerToolCall"; +import { useTheme } from "renderer/stores"; import { type SubagentEntries, toSubagentViewModels, } from "./utils/toSubagentViewModels"; @@ export function SubagentExecutionMessage({ subagents, inline = false, }: SubagentExecutionMessageProps) { + const theme = useTheme(); if (subagents.length === 0) return null; const viewModels = toSubagentViewModels(subagents); @@ {subagent.text ? ( <MessageResponse animated={false} isAnimating={false} - mermaid={{ config: { theme: "default" } }} + mermaid={{ + config: { + theme: theme?.type !== "light" ? "dark" : "default", + }, + }} > {subagent.text} </MessageResponse> ) : null}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/ChatMessageList/components/SubagentExecutionMessage/SubagentExecutionMessage.tsx around lines 83 - 89, The SubagentExecutionMessage currently hardcodes Mermaid config theme to "default" in the MessageResponse usage; update SubagentExecutionMessage to accept and thread the current theme prop (or obtain it the same way MessagePartsRenderer does) and pass that theme into mermaid.config.theme instead of "default", ensuring MessageResponse receives mermaid={{ config: { theme: currentTheme } }} so diagrams respect dark/light mode like MessagePartsRenderer.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPaneInterface/components/ChatMessageList/components/SubagentExecutionMessage/SubagentExecutionMessage.tsx-83-89 (1)
83-89:⚠️ Potential issue | 🟡 MinorDon't hardcode Mermaid to the light theme here.
This renders subagent markdown with
"default"even in dark mode, so diagrams can be inconsistent or hard to read compared with the main message renderer. Thread the current theme through this component the same wayMessagePartsRendererdoes.🎨 Suggested fix
import { Message, MessageContent, MessageResponse, } from "@superset/ui/ai-elements/message"; import { cn } from "@superset/ui/lib/utils"; import { SubagentInnerToolCall } from "renderer/components/Chat/components/SubagentInnerToolCall"; +import { useTheme } from "renderer/stores"; import { type SubagentEntries, toSubagentViewModels, } from "./utils/toSubagentViewModels"; @@ export function SubagentExecutionMessage({ subagents, inline = false, }: SubagentExecutionMessageProps) { + const theme = useTheme(); if (subagents.length === 0) return null; const viewModels = toSubagentViewModels(subagents); @@ {subagent.text ? ( <MessageResponse animated={false} isAnimating={false} - mermaid={{ config: { theme: "default" } }} + mermaid={{ + config: { + theme: theme?.type !== "light" ? "dark" : "default", + }, + }} > {subagent.text} </MessageResponse> ) : null}🤖 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/ChatPane/ChatPaneInterface/components/ChatMessageList/components/SubagentExecutionMessage/SubagentExecutionMessage.tsx` around lines 83 - 89, The Mermaid theme is hardcoded to "default" inside SubagentExecutionMessage (the MessageResponse invocation rendering subagent.text); instead, accept or derive the current UI theme and pass it into the mermaid config the same way MessagePartsRenderer does. Update SubagentExecutionMessage to receive the theme (or read the same theme context/prop used by MessagePartsRenderer) and set mermaid: { config: { theme: /* current theme variable */ } } when rendering MessageResponse so diagrams follow dark/light mode.packages/ui/src/components/ai-elements/clickable-file-path.tsx-43-47 (1)
43-47:⚠️ Potential issue | 🟡 MinorPrevent Space from scrolling while activating the custom button.
On a
role="button"span, Space should callpreventDefault()before invokingonOpen(). Without that, keyboard activation also scrolls the chat viewport.Proposed fix
onKeyDown={(e) => { if (e.key === "Enter" || e.key === " ") { + e.preventDefault(); e.stopPropagation(); onOpen(); } }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ai-elements/clickable-file-path.tsx` around lines 43 - 47, The onKeyDown handler for the role="button" span in ClickableFilePath should call e.preventDefault() when the Space (or legacy "Spacebar") key is pressed before calling onOpen() so the Space activation doesn't scroll the viewport; update the onKeyDown callback (the inline handler that checks e.key === "Enter" || e.key === " ") to also check for "Spacebar" if desired and call e.preventDefault() (then e.stopPropagation() and finally onOpen()).apps/desktop/src/renderer/components/Chat/ChatInterface/components/ChatInputFooter/components/QuestionInputOverlay/QuestionInputOverlay.tsx-44-51 (1)
44-51:⚠️ Potential issue | 🟡 MinorError recovery clears user's custom text input.
When submission fails,
customTextis reset to"". This could be frustrating if the user typed a long response and it gets cleared due to a transient network error.Consider preserving
customTexton error so users can retry without retyping:🔧 Suggested change
const handleSubmitAnswer = (answer: string, label: string) => { if (isDisabled) return; setSubmittedLabel(label); onRespond(question.questionId, answer).catch(() => { setSubmittedLabel(null); - setCustomText(""); + // Keep customText so user can retry without retyping }); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/Chat/ChatInterface/components/ChatInputFooter/components/QuestionInputOverlay/QuestionInputOverlay.tsx` around lines 44 - 51, The catch block in handleSubmitAnswer is clearing the user's typed input by calling setCustomText(""), which loses their work on transient failures; update the error recovery to preserve the current customText (remove the setCustomText("") call) and only reset submittedLabel (or otherwise leave input untouched) so users can retry without retyping — change the catch for onRespond in handleSubmitAnswer to setSubmittedLabel(null) but do not call setCustomText("").apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/SlashCommandNode.tsx-56-66 (1)
56-66:⚠️ Potential issue | 🟡 MinorMissing fallback when
getPos()returnsundefinedfor Backspace.Unlike the Tab/ArrowRight handling which falls back to
editor.commands.focus("end"), the Backspace handler does nothing ifposis undefined. This could leave users unable to delete the chip.🔧 Suggested fix
if (e.key === "Backspace" && args === "") { e.preventDefault(); e.stopPropagation(); const pos = getPos(); if (pos !== undefined) { editor.chain() .focus() .deleteRange({ from: pos, to: pos + node.nodeSize }) .run(); + } else { + // Fallback: attempt to delete via selection if position unavailable + editor.commands.focus("end"); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/SlashCommandNode.tsx` around lines 56 - 66, The Backspace handler for the slash command node fails to fallback when getPos() returns undefined, leaving the chip undeletable; update the Backspace branch (the block referencing getPos(), editor.chain(), and .deleteRange()) to behave like the Tab/ArrowRight handlers by calling a fallback focus when pos is undefined (e.g., call editor.commands.focus("end") or a similar command to move focus to the editor end) so the chip can still be removed or subsequent deletion can occur. Ensure you only call editor.chain().focus().deleteRange(...).run() when pos is defined and otherwise invoke the fallback focus command.packages/ui/src/components/ai-elements/tool.tsx-177-204 (1)
177-204:⚠️ Potential issue | 🟡 MinorDon't hide falsy outputs.
ToolOutputstill treats0,false, and""as “missing” because the guard is truthiness-based. Those are valid tool results and will disappear entirely with this renderer.Suggested change
- if (!(output || errorText)) { + if (output === undefined && errorText === undefined) { return null; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ai-elements/tool.tsx` around lines 177 - 204, The component currently returns null for any falsy output (hiding 0, false, and empty string); update the presence check in ToolOutput so it only treats null/undefined as missing: replace the truthiness guard with a null/undefined check (e.g., if (output == null && errorText == null) return null) so 0, false, and "" render normally; keep the rest of ToolOutput (heading, formatOutput) unchanged.packages/ui/src/components/ai-elements/code-block.tsx-130-142 (1)
130-142:⚠️ Potential issue | 🟡 MinorOnly fade the first span when line numbers are enabled.
With
colorize={false}andshowLineNumbers={false},span:first-childis real code, not the gutter. The first token on every line gets dimmed.Suggested change
className={cn( "overflow-auto dark:hidden [&>pre]:m-0 [&>pre]:bg-background! [&>pre]:p-4 [&>pre]:text-foreground! [&>pre]:text-sm [&_code]:font-mono [&_code]:text-sm", - !colorize && - "[&_span[style]]:!text-foreground [&_.line>span:first-child]:!opacity-50", + !colorize && "[&_span[style]]:!text-foreground", + !colorize && + showLineNumbers && + "[&_.line>span:first-child]:!opacity-50", )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ai-elements/code-block.tsx` around lines 130 - 142, The dimming selector "[&_.line>span:first-child]:!opacity-50" currently applies even when showLineNumbers is false, dimming the first real code token; update the className construction in the code block component (file: code-block.tsx) to only append the gutter-specific selector when showLineNumbers is true (i.e., include "[&_.line>span:first-child]:!opacity-50" conditionally alongside the existing !colorize checks for both dark and light wrappers), so that dimming only occurs when line numbers (gutter) are enabled.
🧹 Nitpick comments (11)
apps/desktop/src/renderer/components/Chat/ChatInterface/hooks/useSlashCommands/useSlashCommands.ts (1)
25-35: Normalizequeryinside the exported matcher.Now that
getCommandMatchRankis exported, Line 25 effectively makes it public API; it currently assumes callers already lowercasedquery. Normalizing at entry avoids case-sensitive misses for external callers.Suggested patch
export function getCommandMatchRank( command: SlashCommand, query: string, ): number | null { - const nameRank = getMatchRank(command.name.toLowerCase(), query); + const normalizedQuery = query.toLowerCase(); + const nameRank = getMatchRank(command.name.toLowerCase(), normalizedQuery); if (nameRank !== null) return nameRank; let bestAliasRank: number | null = null; for (const alias of command.aliases) { - const rank = getMatchRank(alias.toLowerCase(), query); + const rank = getMatchRank(alias.toLowerCase(), normalizedQuery); if (rank === null) continue; const aliasRank = rank + 3; if (bestAliasRank === null || aliasRank < bestAliasRank) { bestAliasRank = aliasRank; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/Chat/ChatInterface/hooks/useSlashCommands/useSlashCommands.ts` around lines 25 - 35, getCommandMatchRank currently assumes the caller lowercases the query; normalize the incoming query at the function entry (e.g., const normalizedQuery = query.toLowerCase()) and use that normalizedQuery when calling getMatchRank for command.name and each alias so external callers won't suffer case-sensitive misses; update references to use normalizedQuery and keep existing behavior of lowercasing command.name/alias as-is.apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/TaskWriteToolCall/TaskWriteToolCall.tsx (1)
13-21: Type guard doesn't validatestatusfield.The
toTodoItemsfilter only checks thatcontentis a string, but theTodoIteminterface expectsstatusto be one of"pending" | "in_progress" | "completed". If incoming data has an invalid or missingstatus, it will pass the filter butbuildDescriptionwill silently count it as none of the categories.♻️ Suggested improvement to validate status
function toTodoItems(value: unknown): TodoItem[] { if (!Array.isArray(value)) return []; + const validStatuses = ["pending", "in_progress", "completed"]; return value.filter( (item): item is TodoItem => typeof item === "object" && item !== null && - typeof (item as TodoItem).content === "string", + typeof (item as TodoItem).content === "string" && + validStatuses.includes((item as TodoItem).status), ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/TaskWriteToolCall/TaskWriteToolCall.tsx` around lines 13 - 21, The type guard toTodoItems currently only checks content but not the TodoItem.status shape, so invalid or missing status values slip through and break buildDescription's categorization; update toTodoItems to also validate that (item as TodoItem).status is one of the allowed strings ("pending", "in_progress", "completed") before narrowing to TodoItem, and ensure the filter excludes entries with missing/invalid status; reference the toTodoItems function, the TodoItem type, and buildDescription when making this change.apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/ChatMessageList/utils/messageListHelpers.ts (1)
104-107: Consider extracting shared filtering logic.Both
getVisibleMessages(lines 104-107) and the fallback inremoveInterruptedSourceMessage(lines 155-160) apply identical filtering: keep non-assistant messages plus assistant messages with answered question tool calls. This could be extracted into a shared helper to reduce duplication.♻️ Proposed refactor to extract shared logic
+function filterActiveTurnMessages(messages: ChatMessage[]): ChatMessage[] { + return messages.filter( + (message) => + message.role !== "assistant" || hasAnsweredQuestionToolCall(message), + ); +} + export function getVisibleMessages({ messages, isRunning, currentMessage, }: { messages: ChatMessage[]; isRunning: boolean; currentMessage: ChatMessage | null; }): ChatMessage[] { if (!isRunning || !currentMessage || currentMessage.role !== "assistant") { return messages; } const turnStartIndex = findLastUserMessageIndex(messages) + 1; const previousTurns = messages.slice(0, turnStartIndex); - const activeTurnNonAssistant = messages - .slice(turnStartIndex) - .filter( - (message) => - message.role !== "assistant" || hasAnsweredQuestionToolCall(message), - ); + const activeTurnNonAssistant = filterActiveTurnMessages( + messages.slice(turnStartIndex), + ); return [...previousTurns, ...activeTurnNonAssistant]; }And similarly update
removeInterruptedSourceMessageto usefilterActiveTurnMessages.Also applies to: 153-161
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/ChatMessageList/utils/messageListHelpers.ts around lines 104 - 107, Extract the duplicated filtering into a shared helper (e.g., filterActiveTurnMessages) and use it from both getVisibleMessages and removeInterruptedSourceMessage: the helper should accept a message array and return messages where message.role !== "assistant" || hasAnsweredQuestionToolCall(message); replace the inline .filter(...) in getVisibleMessages and the fallback filter in removeInterruptedSourceMessage with calls to filterActiveTurnMessages to remove duplication and centralize the logic.apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/SubagentToolCall/SubagentToolCall.tsx (1)
75-85: Add a stable identifier toSubagentToolExecutionor refactor key generation.The
parseSubagentToolResultfunction does not expose a stable call ID in theSubagentToolExecutioninterface—it only providesname,isError,args, andresult. Using${tool.name}-${index}as a key will remount nested rows whenever the tools array is reordered or updated during streaming, which can reset local UI state.Either enhance
parseSubagentToolResultto capture and expose a stable tool call ID (and add it toSubagentToolExecution), or implement a different approach to generate stable keys (e.g., a hash of the tool's args and result).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/SubagentToolCall/SubagentToolCall.tsx` around lines 75 - 85, The current key generation for SubagentInnerToolCall uses a volatile `${tool.name}-${index}` which causes remounts; update parseSubagentToolResult to emit a stable call ID (add a new id or callId field to the SubagentToolExecution interface) and use that id as the React key when rendering SubagentInnerToolCall, or if you prefer not to change parsing, compute a deterministic stable key (e.g., hash of tool.name + JSON.stringify(tool.args) + JSON.stringify(tool.result)) in the mapper and pass that stable key into the SubagentInnerToolCall key prop so nested rows do not remount when order/streaming updates occur.packages/ui/src/components/ai-elements/file-diff-tool.tsx (1)
123-130: Unused state:hasAutoExpandedis set but never read.The
hasAutoExpandedstate is updated in the effect but its value is never used elsewhere in the component. This appears to be leftover from the previous expansion-based implementation.🧹 Suggested removal
export const FileDiffTool = ({ ... }: FileDiffToolProps) => { const hasExpandedRenderer = Boolean(renderExpandedContent); - const [hasAutoExpanded, setHasAutoExpanded] = useState(false); - - useEffect(() => { - if (!hasAutoExpanded && hasExpandedRenderer) { - setHasAutoExpanded(true); - } - }, [hasAutoExpanded, hasExpandedRenderer]); const isStreaming = state === "input-streaming";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ai-elements/file-diff-tool.tsx` around lines 123 - 130, The hasAutoExpanded state and its updater (hasAutoExpanded, setHasAutoExpanded) plus the associated useEffect are unused and should be removed: delete the useState declaration that creates hasAutoExpanded and setHasAutoExpanded, remove the useEffect block that checks hasAutoExpanded and hasExpandedRenderer, and replace any usage of hasExpandedRenderer only where needed (keep the computed hasExpandedRenderer = Boolean(renderExpandedContent)); ensure no other logic depends on hasAutoExpanded or setHasAutoExpanded.apps/desktop/src/renderer/components/Chat/ChatInterface/components/ChatInputFooter/components/QuestionInputOverlay/QuestionInputOverlay.tsx (1)
31-34: Potentially redundantuseEffectfor state reset.The effect resets
submittedLabelandcustomTexton mount, but these are already initialized tonulland""respectively in theuseStatecalls above. If the component is keyed byquestionId(which it is in the parent), it will remount with fresh state anyway.This effect may have been intended to handle cases where the component doesn't remount, but with the current
key={pendingQuestion.questionId}pattern, it's redundant.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/Chat/ChatInterface/components/ChatInputFooter/components/QuestionInputOverlay/QuestionInputOverlay.tsx` around lines 31 - 34, The useEffect that calls setSubmittedLabel(null) and setCustomText("") on mount is redundant because submittedLabel and customText are already initialized via useState and the parent uses key={pendingQuestion.questionId} which causes remounts; remove the useEffect block (the effect that references setSubmittedLabel and setCustomText) to simplify the component, or if you actually intended to reset when questionId changes, replace the empty deps array with [pendingQuestion.questionId] inside the same useEffect.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPaneInterface/components/ChatMessageList/ChatMessageList.tsx (1)
170-182: Unused computed value_interruptedByAbortedQuestion.This
useMemocomputes a boolean that's never used in the component. The underscore prefix suggests it was intentionally marked unused, but this is dead code that adds unnecessary computation.Consider removing it or adding a TODO comment if it's planned for future use.
🤖 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/ChatPane/ChatPaneInterface/components/ChatMessageList/ChatMessageList.tsx` around lines 170 - 182, Remove the dead computed value _interruptedByAbortedQuestion (the useMemo that depends on interruptedPreview and calls normalizeToolName) since it is never referenced; either delete the entire useMemo block or, if you intend to use it later, replace it with a clear TODO comment and keep a minimal placeholder (e.g., a commented note) so no unnecessary computation runs in ChatMessageList.tsx.apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/SlashCommandNode.tsx (1)
17-18: Type assertions onnode.attrscould benefit from defensive defaults.The
node.attrs.nameandnode.attrs.argsare cast without validation. If these attributes are ever missing or have unexpected types, this could cause runtime issues.🛡️ Suggested defensive handling
-const name = node.attrs.name as string; -const args = (node.attrs.args as string) ?? ""; +const name = String(node.attrs.name ?? ""); +const args = String(node.attrs.args ?? "");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/SlashCommandNode.tsx` around lines 17 - 18, The code in SlashCommandNode.tsx unsafely asserts node.attrs.name and node.attrs.args; update the assignments to defensively validate types on node.attrs (e.g., check typeof node.attrs.name === 'string' and typeof node.attrs.args === 'string') and provide safe defaults if missing or wrong type (e.g., name fallback to empty string or a sensible default, args to empty string) so that the variables used later are always strings and avoid runtime errors.apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/AskUserQuestionToolCall/AskUserQuestionToolCall.tsx (1)
30-31: UnusedisStreamingprop.The
isStreamingprop is defined in the interface but not destructured or used in the component body (onlyisInterruptedis used at line 178). Consider removing it if not needed, or verify if it should be used in the pending/status logic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/AskUserQuestionToolCall/AskUserQuestionToolCall.tsx` around lines 30 - 31, The AskUserQuestionToolCall component defines an unused prop isStreaming in its props/interface while only isInterrupted is referenced; either remove isStreaming from the props/interface and any upstream callers that pass it, or update the component (e.g., in AskUserQuestionToolCall's render/logic where pending/status are computed) to destructure and use isStreaming alongside isInterrupted to control streaming-specific UI/behavior. Locate the AskUserQuestionToolCall component and its props/interface declaration and make the minimal change: delete the isStreaming prop if not required, or add isStreaming to the destructuring and incorporate it into the pending/status logic where isInterrupted is currently used.apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/TiptapPromptEditor.tsx (2)
27-29: Import statement placed after variable declarations.The import on line 29 appears after the
PluginKeydeclarations on lines 27-28. While syntactically valid, this is unconventional and may confuse readers or trigger linter warnings in some configurations. Consider moving all imports to the top of the file.♻️ Suggested reordering
import { PluginKey } from "@tiptap/pm/state"; import { EditorContent, useEditor } from "@tiptap/react"; import Suggestion from "@tiptap/suggestion"; +import { useEffect, useLayoutEffect, useRef, useState } from "react"; +import { useDebouncedValue } from "renderer/hooks/useDebouncedValue"; +import { FileIcon } from "renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/utils"; const slashSuggestionKey = new PluginKey("slashCommandSuggestion"); const mentionSuggestionKey = new PluginKey("fileMentionSuggestion"); -import { useEffect, useLayoutEffect, useRef, useState } from "react"; -import { useDebouncedValue } from "renderer/hooks/useDebouncedValue"; -import { FileIcon } from "renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/utils";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/TiptapPromptEditor.tsx` around lines 27 - 29, Move all import statements to the top of the file so they appear before any runtime declarations; specifically relocate the import that currently appears after the PluginKey declarations (the import bringing in React hooks like useEffect/useRef/useState) above the lines that create slashSuggestionKey and mentionSuggestionKey (which use PluginKey) so that PluginKey and those constant declarations follow the file-level imports in TiptapPromptEditor.tsx.
157-165: Effect dependency on fullmentionStateobject may cause unnecessary runs.The effect depends on
mentionStatebut only readsmentionState.selectedIndex. Any change tomentionState(query, clientRect, tiptapCommand) will re-run this effect, though the guards prevent actual state updates in most cases. Consider extractingmentionState?.selectedIndexto a variable and using that in the dependency array for cleaner semantics.♻️ Suggested optimization
+ const mentionSelectedIndex = mentionState?.selectedIndex ?? 0; + // Clamp selectedIndex when file results shrink useEffect(() => { - if (!mentionState || mentionFiles.length === 0) return; + if (mentionState === null || mentionFiles.length === 0) return; const max = mentionFiles.length - 1; - if (mentionState.selectedIndex > max) { + if (mentionSelectedIndex > max) { setMentionState((prev) => prev ? { ...prev, selectedIndex: max } : null, ); } - }, [mentionFiles.length, mentionState]); + }, [mentionFiles.length, mentionState, mentionSelectedIndex]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/TiptapPromptEditor.tsx` around lines 157 - 165, The effect currently depends on the whole mentionState object causing unnecessary runs; change the dependency to only the selected index by deriving a local variable like const selectedIndex = mentionState?.selectedIndex and use selectedIndex (and mentionFiles.length) in the useEffect dependency array, while keeping the same guard logic and calls to setMentionState to clamp selectedIndex when it exceeds max (referencing mentionState, mentionFiles, setMentionState).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 30189ced-7006-4f1a-8989-e4ebdaeb6c21
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (83)
AGENTS.mdapps/desktop/src/main/lib/notifications/notification-manager.tsapps/desktop/src/main/windows/main.tsapps/desktop/src/renderer/components/Chat/ChatInterface/components/ChatInputFooter/ChatInputFooter.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/ChatInputFooter/components/QuestionInputOverlay/QuestionInputOverlay.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/ChatInputFooter/components/QuestionInputOverlay/index.tsapps/desktop/src/renderer/components/Chat/ChatInterface/components/MessageList/MessageList.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/MessagePartsRenderer/MessagePartsRenderer.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/ReadOnlyToolCall/ReadOnlyToolCall.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/SlashCommandMenu/SlashCommandMenu.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/FileMentionNode.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/SlashCommandNode.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/SlashCommandPreviewPopover.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/TiptapPromptEditor.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/index.tsapps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/parseTextToEditorContent.tsapps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/serializeEditorToText.tsapps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/ToolCallBlock.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/AskUserQuestionToolCall/AskUserQuestionToolCall.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/GenericToolCall/GenericToolCall.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/GenericToolCall/getGenericToolCallState.tsapps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/LspInspectToolCall/LspInspectToolCall.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/LspInspectToolCall/index.tsapps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/SkillToolCall/SkillToolCall.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/SkillToolCall/index.tsapps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/SubagentToolCall/SubagentToolCall.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/SubagentToolCall/utils/parseSubagentToolResult/parseSubagentToolResult.tsapps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/SupersetToolCall/SupersetToolCall.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/TaskWriteToolCall/TaskWriteToolCall.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/TaskWriteToolCall/index.tsapps/desktop/src/renderer/components/Chat/ChatInterface/hooks/useFocusPromptOnPane.tsapps/desktop/src/renderer/components/Chat/ChatInterface/hooks/useSlashCommands/index.tsapps/desktop/src/renderer/components/Chat/ChatInterface/hooks/useSlashCommands/useSlashCommands.tsapps/desktop/src/renderer/components/Chat/ChatInterface/utils/messageHelpers.tsapps/desktop/src/renderer/components/Chat/ChatInterface/utils/tool-helpers.tsapps/desktop/src/renderer/components/Chat/components/MarkdownToggleContent/MarkdownToggleContent.tsxapps/desktop/src/renderer/components/Chat/components/MarkdownToggleContent/index.tsapps/desktop/src/renderer/components/Chat/components/SubagentInnerToolCall/SubagentInnerToolCall.tsxapps/desktop/src/renderer/components/Chat/components/SubagentInnerToolCall/index.tsapps/desktop/src/renderer/components/MarkdownRenderer/components/CodeBlock/CodeBlock.tsxapps/desktop/src/renderer/globals.cssapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/ChatPaneInterface.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/ChatInputFooter/ChatInputFooter.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/ChatMessageList/ChatMessageList.test.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/ChatMessageList/ChatMessageList.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/ChatMessageList/ChatMessageList.types.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/ChatMessageList/components/AssistantMessage/AssistantMessage.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/ChatMessageList/components/SubagentExecutionMessage/SubagentExecutionMessage.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/ChatMessageList/components/SubagentExecutionMessage/utils/toSubagentViewModels.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/ChatMessageList/utils/messageListHelpers.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/utils/sendMessage/sendMessage.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/hooks/useWorkspaceChatDisplay/useWorkspaceChatDisplay.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPaneInterface/ChatPaneInterface.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPaneInterface/components/ChatMessageList/ChatMessageList.test.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPaneInterface/components/ChatMessageList/ChatMessageList.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPaneInterface/components/ChatMessageList/ChatMessageList.types.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPaneInterface/components/ChatMessageList/components/AssistantMessage/AssistantMessage.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPaneInterface/components/ChatMessageList/components/SubagentExecutionMessage/SubagentExecutionMessage.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPaneInterface/components/ChatMessageList/components/SubagentExecutionMessage/utils/toSubagentViewModels.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPaneInterface/components/ChatMessageList/utils/messageListHelpers.tsapps/desktop/src/renderer/stores/tabs/useAgentHookListener.tsapps/desktop/src/shared/notification-types.tsdocs/skill-preload-feature.mdpackages/chat/src/client/hooks/use-chat-display/use-chat-display.tspackages/chat/src/server/trpc/service.tspackages/chat/src/server/trpc/utils/runtime/runtime.tspackages/chat/src/server/trpc/zod.tspackages/host-service/src/runtime/chat/chat.tspackages/ui/src/components/ai-elements/bash-tool.tsxpackages/ui/src/components/ai-elements/braille-spinner.tsxpackages/ui/src/components/ai-elements/clickable-file-path.tsxpackages/ui/src/components/ai-elements/code-block.tsxpackages/ui/src/components/ai-elements/conversation.tsxpackages/ui/src/components/ai-elements/file-diff-tool.tsxpackages/ui/src/components/ai-elements/message.tsxpackages/ui/src/components/ai-elements/prompt-input.tsxpackages/ui/src/components/ai-elements/read-file-tool.tsxpackages/ui/src/components/ai-elements/show-code.tsxpackages/ui/src/components/ai-elements/tool-call-row.tsxpackages/ui/src/components/ai-elements/tool.tsxpackages/ui/src/components/ai-elements/web-fetch-tool.tsxpackages/ui/src/components/ai-elements/web-search-tool.tsxpackages/ui/src/components/ui/input-group.tsx
💤 Files with no reviewable changes (3)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/ChatMessageList/ChatMessageList.test.tsx
- apps/desktop/src/renderer/components/Chat/components/MarkdownToggleContent/index.ts
- apps/desktop/src/renderer/components/Chat/components/MarkdownToggleContent/MarkdownToggleContent.tsx
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
25 issues found across 84 files
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed. cubic prioritises the most important files to review.
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/ui/src/components/ai-elements/clickable-file-path.tsx">
<violation number="1" location="packages/ui/src/components/ai-elements/clickable-file-path.tsx:27">
P2: Basename extraction is POSIX-only (`/` split), so Windows paths render full path instead of filename.</violation>
</file>
<file name="apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/FileMentionNode.tsx">
<violation number="1" location="apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/FileMentionNode.tsx:11">
P2: Filename extraction uses POSIX-only splitting, causing Windows-style file mention paths to render incorrectly.</violation>
</file>
<file name="apps/desktop/src/renderer/components/Chat/ChatInterface/utils/messageHelpers.ts">
<violation number="1" location="apps/desktop/src/renderer/components/Chat/ChatInterface/utils/messageHelpers.ts:37">
P2: Duplicate parsing/business logic in both question-status helpers should be centralized to avoid behavior drift.</violation>
</file>
<file name="apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/SupersetToolCall/SupersetToolCall.tsx">
<violation number="1" location="apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/SupersetToolCall/SupersetToolCall.tsx:59">
P2: Using `content ?? text` can hide valid output when `content` is empty/whitespace and `text` has the real result.</violation>
</file>
<file name="apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/LspInspectToolCall/LspInspectToolCall.tsx">
<violation number="1" location="apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/LspInspectToolCall/LspInspectToolCall.tsx:23">
P2: Basename extraction uses `split("/")`, which is not cross-platform and mis-handles Windows paths.</violation>
</file>
<file name="packages/host-service/src/runtime/chat/chat.ts">
<violation number="1" location="packages/host-service/src/runtime/chat/chat.ts:378">
P2: Filesystem/setup errors are silently swallowed in `ensureGlobalAgentInstructions`, hiding failures in a newly added runtime initialization path.</violation>
</file>
<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPaneInterface/components/ChatMessageList/ChatMessageList.tsx">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPaneInterface/components/ChatMessageList/ChatMessageList.tsx:170">
P2: Unused memoized variable introduces dead code and unnecessary render-time computation.</violation>
</file>
<file name="apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/TaskWriteToolCall/TaskWriteToolCall.tsx">
<violation number="1" location="apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/TaskWriteToolCall/TaskWriteToolCall.tsx:48">
P2: Unsafe dereference of `args.todos` relies on `getArgs` returning an object, but `getArgs` can return non-object parsed JSON values at runtime.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/ChatMessageList/components/SubagentExecutionMessage/SubagentExecutionMessage.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/ChatMessageList/components/SubagentExecutionMessage/SubagentExecutionMessage.tsx:83">
P2: Subagent text rendering lost the previous max-height/overflow containment, allowing large outputs to expand unbounded and potentially degrade chat layout/render performance.</violation>
</file>
<file name="apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/SlashCommandPreviewPopover.tsx">
<violation number="1" location="apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/SlashCommandPreviewPopover.tsx:55">
P2: Popover anchor rect is only recalculated on input/editor changes, so scroll/resize/layout shifts can leave the preview popover visually detached from the slash-command chip.</violation>
</file>
<file name="packages/ui/src/components/ai-elements/code-block.tsx">
<violation number="1" location="packages/ui/src/components/ai-elements/code-block.tsx:80">
P1: `highlightCode` can recurse indefinitely because the catch-all fallback retries by calling itself without a base-case guard.</violation>
<violation number="2" location="packages/ui/src/components/ai-elements/code-block.tsx:133">
P2: `colorize=false` incorrectly fades the first code token when line numbers are disabled because the first-child opacity rule is applied unconditionally.</violation>
</file>
<file name="apps/desktop/src/renderer/stores/tabs/useAgentHookListener.ts">
<violation number="1" location="apps/desktop/src/renderer/stores/tabs/useAgentHookListener.ts:94">
P2: Stop status override uses `pane.status === "permission"` as a proxy for PendingQuestion, but PermissionRequest shares that same status, so inactive permission flows are incorrectly forced to `idle` instead of `review`.</violation>
</file>
<file name="packages/chat/src/server/trpc/zod.ts">
<violation number="1" location="packages/chat/src/server/trpc/zod.ts:90">
P2: `skills` was added only to `sendMessage` metadata, but restart still uses the old metadata contract and runtime path, so restart flows cannot apply preloaded skills like normal sends.</violation>
</file>
<file name="apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/parseTextToEditorContent.ts">
<violation number="1" location="apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/parseTextToEditorContent.ts:9">
P2: Mention round-trip format is ambiguous: `/@(\S+)/g` reinterprets literal `@` text as mentions and truncates mention paths containing spaces.</violation>
</file>
<file name="apps/desktop/src/renderer/components/Chat/ChatInterface/components/ChatInputFooter/components/QuestionInputOverlay/QuestionInputOverlay.tsx">
<violation number="1" location="apps/desktop/src/renderer/components/Chat/ChatInterface/components/ChatInputFooter/components/QuestionInputOverlay/QuestionInputOverlay.tsx:34">
P2: Per-question transient state is only reset on mount, so reused overlay instances can retain stale submission/input state across question changes.</violation>
</file>
<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPaneInterface/components/ChatMessageList/components/SubagentExecutionMessage/utils/toSubagentViewModels.ts">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPaneInterface/components/ChatMessageList/components/SubagentExecutionMessage/utils/toSubagentViewModels.ts:95">
P2: `args` is cast to `Record<string, unknown>` after a broad object check, allowing arrays/non-record objects through and breaking the declared shape contract.</violation>
<violation number="2" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPaneInterface/components/ChatMessageList/components/SubagentExecutionMessage/utils/toSubagentViewModels.ts:101">
P2: Structured `toolCalls.result` values are lossily coerced with `String(...)`, which can destroy object/array output fidelity before UI rendering.</violation>
</file>
<file name="packages/ui/src/components/ai-elements/read-file-tool.tsx">
<violation number="1" location="packages/ui/src/components/ai-elements/read-file-tool.tsx:54">
P2: ReadFileTool does not plumb `startLine` to `ShowCode`, so line-number gutters can be incorrect for partial-file ranges.</violation>
</file>
<file name="apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/SubagentToolCall/utils/parseSubagentToolResult/parseSubagentToolResult.ts">
<violation number="1" location="apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/SubagentToolCall/utils/parseSubagentToolResult/parseSubagentToolResult.ts:35">
P2: Detailed tool-call parsing is format-fragile because the regex requires a leading newline before `<subagent-tool-calls>`, causing valid blocks at string start to be ignored and potentially leaked into rendered text.</violation>
<violation number="2" location="apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/SubagentToolCall/utils/parseSubagentToolResult/parseSubagentToolResult.ts:64">
P2: Empty catch around detailed tool-call JSON parsing silently swallows failures, hiding serializer/contract regressions and making parsing issues non-observable.</violation>
</file>
<file name="apps/desktop/src/renderer/components/Chat/ChatInterface/components/ReadOnlyToolCall/ReadOnlyToolCall.tsx">
<violation number="1" location="apps/desktop/src/renderer/components/Chat/ChatInterface/components/ReadOnlyToolCall/ReadOnlyToolCall.tsx:89">
P2: Read-file UI mixes full-file disk content with range-based labels, causing misleading line ranges when range args are present.</violation>
</file>
<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPaneInterface/components/ChatMessageList/utils/messageListHelpers.ts">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPaneInterface/components/ChatMessageList/utils/messageListHelpers.ts:109">
P2: Duplicated assistant-visibility predicate in two paths should be extracted to a shared helper to prevent behavior drift.</violation>
<violation number="2" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPaneInterface/components/ChatMessageList/utils/messageListHelpers.ts:160">
P2: Fallback dedup removes all non-question assistant messages in the active turn when ID match fails, which can delete legitimate assistant history unrelated to the interrupted source.</violation>
</file>
<file name="apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/TiptapPromptEditor.tsx">
<violation number="1" location="apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/TiptapPromptEditor.tsx:473">
P2: Enter/Tab are swallowed in file-mention mode even when no result exists, preventing normal Enter submit while mention state is active.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ui/src/components/ai-elements/bash-tool.tsx (1)
43-44:⚠️ Potential issue | 🟡 MinorTreat
output-erroras an error even without an exit code.
BashToolStatealready has an explicit"output-error"branch, butisErroronly looks atexitCode. Failures that do not produce an exit code will render as a normal Bash row.🧮 Safer error derivation
- const isError = exitCode !== undefined && exitCode !== 0; + const isError = state === "output-error" || (exitCode !== undefined && exitCode !== 0);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ai-elements/bash-tool.tsx` around lines 43 - 44, The current error flag only checks exitCode, so rows with state "output-error" are not treated as errors; update the derivation of isError (used alongside isPending) to also consider state === "output-error" or otherwise check BashToolState for the "output-error" branch so that any output-error state is treated as an error even when exitCode is undefined (referring to the isError variable and the state/exitCode values in bash-tool.tsx).
♻️ Duplicate comments (8)
packages/host-service/src/runtime/chat/chat.ts (1)
354-380:⚠️ Potential issue | 🟠 MajorDon't enforce Superset policy by mutating the user's global
~/.mastracode/AGENTS.md.This leaks a workspace-specific rule into every other Mastra session on the machine, and the unmanaged-file early return means Superset still won't reliably enforce
ask_userhere. Inject these instructions into the runtime/session you create instead of writing shared user-global state.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/src/runtime/chat/chat.ts` around lines 354 - 380, The ensureGlobalAgentInstructions method currently writes Superset-specific rules into a user-global file (AGENTS.md) which must be removed; stop mutating ~/.mastracode/AGENTS.md by deleting or turning ensureGlobalAgentInstructions into a no-op and remove uses of MANAGED_MARKER/INSTRUCTIONS that perform filesystem writes, and instead inject the same instruction text (INSTRUCTIONS) into the runtime/session-level configuration used when creating the chat runtime or agent (e.g., include the instruction in the system prompt or session-specific agent instructions where the chat/session is constructed), so the ask_user policy is enforced per-session without altering global user files.apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/parseTextToEditorContent.ts (2)
26-35:⚠️ Potential issue | 🟠 MajorRewrite the
exec()loop so Biome passes.The assignment inside
while ((match = MENTION_RE.exec(line)) !== null)triggerslint/suspicious/noAssignInExpressions, so this file will fail lint as written.♻️ Lint-safe rewrite
- let match: RegExpExecArray | null; - while ((match = MENTION_RE.exec(line)) !== null) { + let match = MENTION_RE.exec(line); + while (match !== null) { // Text before the mention if (match.index > lastIndex) { inlineNodes.push({ type: "text", text: line.slice(lastIndex, match.index) }); } // The file-mention node inlineNodes.push({ type: "file-mention", attrs: { path: match[1] } }); lastIndex = match.index + match[0].length; + match = MENTION_RE.exec(line); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/parseTextToEditorContent.ts` around lines 26 - 35, The loop uses an assignment inside the while condition which trips the lint rule; refactor the loop so you call MENTION_RE.exec(line) into the local variable match before checking it and break when null. Concretely, in the function that contains MENTION_RE, replace the while ((match = MENTION_RE.exec(line)) !== null) pattern with an explicit loop that sets match = MENTION_RE.exec(line) at the top (or in a for(;;) loop) and breaks when match === null, then keep the existing body that pushes text nodes and the file-mention node to inlineNodes and updates lastIndex (referencing MENTION_RE, match, inlineNodes, lastIndex, and the file-mention attrs).
9-45:⚠️ Potential issue | 🟠 MajorMake the parser match
serializeEditorToText()'s actual wire format.This function says it round-trips serialized editor text, but it only matches
@(\S+). That means file mentions serialized as@{path}come back with the braces inattrs.path, ordinary text likefoo@bar.comgets rewritten into afile-mention, and serialized slash-command chips (/{name} {args}) are never restored at all. Draft rehydration needs an unambiguous parser for the real serialized tokens.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/parseTextToEditorContent.ts` around lines 9 - 45, The parser parseTextToEditorContent currently uses MENTION_RE = /@(\S+)/g which incorrectly captures emails and leaves braces in attrs.path and ignores slash-command chips; update the parsing logic in parseTextToEditorContent to (1) detect the exact wire token forms produced by serializeEditorToText: file mentions in the form @{path} (capture path without braces) and standalone `@path` tokens only when they match the serializer's unambiguous pattern (so do not treat email-like tokens as mentions), and (2) detect slash-command chips serialized as "/{name} {args}" and emit a node (e.g., type "slash-command" with attrs name and args). Replace MENTION_RE with a regex that recognizes the @{...} form (and only the allowed unbraced form per serializer rules) and add a SLASH_RE for "/{name} {args}", then update the while/loop in parseTextToEditorContent to branch on which capture matched and push the appropriate { type: "file-mention", attrs: { path } } or { type: "slash-command", attrs: { name, args } } nodes, leaving ordinary text (e.g., foo@bar.com) untouched.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPaneInterface/ChatPaneInterface.tsx (1)
981-1002:⚠️ Potential issue | 🟠 MajorRollback the optimistic question state when
respondToQuestion()fails.
answeredQuestionIdand the pane"idle"status are flipped before the RPC resolves. If the request rejects, Lines 480-485 never clear the same question id, so the overlay stays hidden and the orange dot remains cleared while the question is still pending.🩹 Minimal fix
const handleQuestionResponse = useCallback( async (questionId: string, answer: string) => { const trimmedQuestionId = questionId.trim(); const trimmedAnswer = answer.trim(); if (!trimmedQuestionId || !trimmedAnswer) return; clearRuntimeError(); - setAnsweredQuestionId(trimmedQuestionId); setQuestionResponsePending(true); - // Clear the orange dot immediately when the user submits their answer - useTabsStore.getState().setPaneStatus(paneId, "idle"); try { await commands.respondToQuestion({ payload: { questionId: trimmedQuestionId, answer: trimmedAnswer, }, }); + setAnsweredQuestionId(trimmedQuestionId); + useTabsStore.getState().setPaneStatus(paneId, "idle"); } finally { setQuestionResponsePending(false); } },🤖 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/ChatPane/ChatPaneInterface/ChatPaneInterface.tsx` around lines 981 - 1002, Before optimistically flipping state in handleQuestionResponse, capture the previous pane status and the previous answeredQuestionId, then wrap the respondToQuestion call in try/catch; on rejection restore answeredQuestionId (via setAnsweredQuestionId) and restore the pane status (via useTabsStore.getState().setPaneStatus(paneId, previousStatus) or at minimum set it back to the pending/question status), and still clear setQuestionResponsePending in finally; reference handleQuestionResponse, answeredQuestionId/setAnsweredQuestionId, useTabsStore.getState().setPaneStatus, and commands.respondToQuestion when making the change.packages/ui/src/components/ai-elements/show-code.tsx (1)
126-200:⚠️ Potential issue | 🟡 MinorAdd accessible names to the icon-only buttons.
These buttons only render icons and lack
aria-labelattributes. Tooltips are not exposed to screen readers in the same way, leaving these controls unlabeled for assistive technology users.♿ Proposed fix: add aria-label to each button
<Button + aria-label={isExpanded ? "Collapse code" : "Expand code"} className="h-6 w-6" onClick={() => setIsExpanded((prev) => !prev)} size="icon" variant="ghost" > ... <Button + aria-label="Open file" className="h-6 w-6" onClick={(e) => { e.stopPropagation(); onOpen(); }} size="icon" variant="ghost" > ... <Button + aria-label={isCopied ? "Copied" : "Copy code"} className="h-6 w-6" onClick={handleCopy} size="icon" variant="ghost" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ai-elements/show-code.tsx` around lines 126 - 200, The icon-only Buttons in the ShowCode component are missing accessible names; add aria-label attributes to each Button: for the expand/collapse toggle Button (used with setIsExpanded and isExpanded when isOverflowing) set aria-label to either "Collapse" or "Expand" based on isExpanded; for the external open Button (onClick calling onOpen when filename is present) set aria-label to "Open" or include the filename like `Open ${filename}`; and for the copy Button (onClick -> handleCopy) set aria-label to "Copy code" (or similar). Ensure you update the Button props where those Buttons are rendered so screen readers receive a clear label.packages/ui/src/components/ai-elements/code-block.tsx (1)
67-88:⚠️ Potential issue | 🟠 MajorGuard the Shiki fallback against non-language failures.
The catch block retries with
"text"for every exception. IfcodeToHtmlfails due to Shiki initialization or theme loading issues—rather than an unsupported language—the"text"path will throw identically and recurse until stack overflow. Add a recursion guard or distinguish language errors.🐛 Proposed fix: add a retry flag
export async function highlightCode( code: string, language: BundledLanguage, showLineNumbers = false, startLine = 1, + _isRetry = false, ) { const transformers: ShikiTransformer[] = showLineNumbers ? [createLineNumberTransformer(startLine)] : []; try { return await Promise.all([ codeToHtml(code, { lang: language, theme: "one-light", transformers, }), codeToHtml(code, { lang: language, theme: "one-dark-pro", transformers, }), ]); } catch { + if (_isRetry) { + // Already retried with "text" — return unhighlighted fallback + const escaped = code.replace(/[<>&]/g, (c) => + c === "<" ? "<" : c === ">" ? ">" : "&", + ); + const html = `<pre><code>${escaped}</code></pre>`; + return [html, html]; + } // Unknown/unsupported language — fall back to plain text return highlightCode( code, "text" as BundledLanguage, showLineNumbers, startLine, + true, ); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ai-elements/code-block.tsx` around lines 67 - 88, The catch currently retries by calling highlightCode with "text" for every exception which can recurse if the failure is from shiki/theme init; modify the try/catch in code-block.tsx around codeToHtml so it distinguishes unsupported-language errors from other failures or adds a one-time retry guard (e.g., a boolean retryAttempted local flag or parameter) before falling back to highlightCode; specifically update the block that calls codeToHtml and the fallback call to highlightCode so that if the fallback itself fails you surface or rethrow the original error instead of recursively retrying.apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/AskUserQuestionToolCall/AskUserQuestionToolCall.tsx (1)
231-251:⚠️ Potential issue | 🔴 CriticalHook called conditionally after early return — violates Rules of Hooks.
The
useMemoat line 239 is called after the early return at lines 231-232. React requires hooks to be called in the same order on every render. This will cause runtime errors when the early return condition changes between renders.The past review comment indicated this was addressed, but the issue persists in the current code.
🐛 Proposed fix: Move useMemo before the early return
+ const answeredQAs = useMemo( + () => + questions + .map((q) => ({ + question: q.question, + answer: findAnswerForQuestion({ answers, questionText: q.question }), + })) + .filter( + (qa): qa is { question: string; answer: string } => + qa.answer !== undefined, + ), + [questions, answers], + ); + // No args available (tool_result-only path with input: {}) — nothing useful to show if (questions.length === 0 && !isCancelledByError && !isCancelledByStop) return null; const isAnswered = !isPending && !isCancelledByError && !isCancelledByStop && hasAnswers; const isCancelled = !isPending && !isCancelledByError && !isCancelledByStop && !hasAnswers; - const answeredQAs = useMemo( - () => - questions - .map((q) => ({ - question: q.question, - answer: findAnswerForQuestion({ answers, questionText: q.question }), - })) - .filter( - (qa): qa is { question: string; answer: string } => - qa.answer !== undefined, - ), - [questions, answers], - );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/AskUserQuestionToolCall/AskUserQuestionToolCall.tsx` around lines 231 - 251, The useMemo hook (creating answeredQAs) is called after an early return which violates the Rules of Hooks; move the useMemo block that computes answeredQAs (using useMemo, answeredQAs, questions, answers, and findAnswerForQuestion) above the early return that checks questions.length, isCancelledByError, and isCancelledByStop so hooks are executed unconditionally and maintain stable order across renders; keep the same dependencies ([questions, answers]) and preserve the filtering logic and type guard.apps/desktop/src/renderer/components/Chat/components/SubagentInnerToolCall/SubagentInnerToolCall.tsx (1)
217-246:⚠️ Potential issue | 🔴 CriticalMove
useMemobefore the early return to fix React hook order violation.The
useMemoat line 240 is called after an early return at line 217. Ifnormalizedchanges between"mastra_workspace_execute_command"and another tool across renders, hooks will be called in a different order, violating React's rules of hooks and causing a runtime error.🐛 Proposed fix
export function SubagentInnerToolCall({ name, isError, isPending = false, args, result, workspaceCwd, onOpenFileInPane, }: SubagentInnerToolCallProps) { const normalized = normalizeToolName(name); const state = isPending ? ("input-available" as const) : isError ? ("output-error" as const) : ("output-available" as const); + const { label, icon } = getToolMeta(normalized); + const description = getDescription(normalized, args); + const hasResult = result !== null && result.trim().length > 0; + + // Read file: parse result for the shared ReadFileTool component + const parsedReadFile = useMemo( + () => + normalized === "mastra_workspace_read_file" && hasResult + ? parseReadFileResult(result!) + : null, + [normalized, hasResult, result], + ); + if (normalized === "mastra_workspace_execute_command") { const argsRecord = args ?? {}; const resultRecord = result !== null ? { content: result } : {}; const { command, stdout, stderr, exitCode } = getExecuteCommandViewModel({ args: argsRecord, result: resultRecord, }); return ( <BashTool command={command} stdout={stdout} stderr={stderr} exitCode={exitCode} state={state} /> ); } - const { label, icon } = getToolMeta(normalized); - const description = getDescription(normalized, args); - const hasResult = result !== null && result.trim().length > 0; - - // Read file: parse and display using the shared ReadFileTool component - const parsedReadFile = useMemo( - () => - normalized === "mastra_workspace_read_file" && hasResult - ? parseReadFileResult(result!) - : null, - [normalized, hasResult, result], - ); if ( normalized === "mastra_workspace_read_file" && hasResult && parsedReadFile ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/Chat/components/SubagentInnerToolCall/SubagentInnerToolCall.tsx` around lines 217 - 246, The useMemo call (creating parsedReadFile via parseReadFileResult) is placed after an early return for the mastra_workspace_execute_command branch, causing a hooks order violation when normalized changes; move the useMemo block above the conditional that returns the BashTool so all hooks run unconditionally in the same order (keep the same dependency array [normalized, hasResult, result] and logic checking normalized === "mastra_workspace_read_file" && hasResult), then retain the existing early return that renders BashTool for normalized === "mastra_workspace_execute_command".
🧹 Nitpick comments (7)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPaneInterface/components/ChatMessageList/utils/messageListHelpers.ts (1)
107-112: Extract the active-turn question-visibility predicate.This
answered || pendingcondition now lives in two functions here, and the sibling workspace helper has already drifted. Pulling it behind one helper will make futureask_uservisibility changes much harder to miss.Possible shape
+function shouldKeepActiveTurnMessage(message: ChatMessage): boolean { + return ( + message.role !== "assistant" || + hasAnsweredQuestionToolCall(message) || + hasPendingQuestionToolCall(message) + ); +} ... const activeTurnNonAssistant = messages .slice(turnStartIndex) - .filter( - (message) => - message.role !== "assistant" || - hasAnsweredQuestionToolCall(message) || - hasPendingQuestionToolCall(message), - ); + .filter(shouldKeepActiveTurnMessage); ... const activeTurnFiltered = messages .slice(turnStartIndex) - .filter( - (message) => - message.role !== "assistant" || - hasAnsweredQuestionToolCall(message) || - hasPendingQuestionToolCall(message), - ); + .filter(shouldKeepActiveTurnMessage);Also applies to: 162-167
🤖 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/ChatPane/ChatPaneInterface/components/ChatMessageList/utils/messageListHelpers.ts` around lines 107 - 112, The duplicated predicate that decides if an assistant message with an ask_user tool call should be visible (currently written as message.role !== "assistant" || hasAnsweredQuestionToolCall(message) || hasPendingQuestionToolCall(message)) is repeated in this file (and similar logic at lines 162-167); extract that logic into a single helper (e.g., isAskUserVisibleForMessage or isActiveTurnQuestionVisible) and replace both inline uses with a call to that helper, referencing the existing helpers hasAnsweredQuestionToolCall and hasPendingQuestionToolCall inside it so visibility rules are centralized.packages/ui/src/components/ai-elements/file-diff-tool.tsx (1)
123-130: Remove unusedhasAutoExpandedstate and effect.The
hasAutoExpandedstate is set but never read to control any behavior. This appears to be leftover from the removed expansion logic.🧹 Remove dead code
export const FileDiffTool = ({ // ... props }: FileDiffToolProps) => { const hasExpandedRenderer = Boolean(renderExpandedContent); - const [hasAutoExpanded, setHasAutoExpanded] = useState(false); - - useEffect(() => { - if (!hasAutoExpanded && hasExpandedRenderer) { - setHasAutoExpanded(true); - } - }, [hasAutoExpanded, hasExpandedRenderer]); const isStreaming = state === "input-streaming";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ai-elements/file-diff-tool.tsx` around lines 123 - 130, Remove the dead state and effect: delete the useState declaration for hasAutoExpanded and its setter (setHasAutoExpanded) and remove the useEffect that checks hasAutoExpanded and hasExpandedRenderer; the only remaining signal is hasExpandedRenderer (derived from renderExpandedContent) so eliminating hasAutoExpanded/useEffect cleans up unused state and side-effect logic in file-diff-tool.tsx.apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/ChatPaneInterface.tsx (1)
534-553: Skill extraction may unintentionally modify message content.The regex replaces
/tokenwithtoken(stripping the/) for custom commands, but returns the originalmatchfor non-custom commands. This means:
- Built-in commands at the start are handled by
resolveSlashCommandInputlater (correct).- Non-command
/patternsmid-text (e.g., URLs likeexample.com/path) are preserved (correct).- Custom command chips anywhere in text get their
/stripped (intended).However, if a user types a custom command name without intending to invoke it (e.g., discussing the
/redesigncommand), it will still be extracted and stripped.Consider whether skill extraction should only match explicit chip syntax (e.g., from the Tiptap editor) rather than raw
/commandpatterns in text, to avoid false positives from conversational mentions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/ChatPaneInterface.tsx around lines 534 - 553, The current extraction using /\/(\S+)/g (producing contentWithSkillsExtracted, skillNames and using findSlashCommandByNameOrAlias against slashCommands) strips leading slashes even when users are merely mentioning a command, causing false positives; change the extraction to only recognize explicit chip/token syntax from the editor (e.g., Tiptap node marks or a serialized chip token) rather than a generic "/word" regex—update the logic in the component that builds contentWithSkillsExtracted to query the editor/serialized node tree or match the editor's chip marker (or require a preceding whitespace/start-of-line and trailing boundary) and only then push to skillNames, leaving all other raw text unchanged so resolveSlashCommandInput still handles builtin commands correctly.apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/SupersetToolCall/SupersetToolCall.tsx (1)
55-63: Consider usinguseMemoforcontentTextfor consistency.The
errorTextderivation usesuseMemo(lines 46-53), butcontentTextuses an IIFE. For consistency and to avoid recalculation on every render, consider wrappingcontentTextinuseMemoas well.♻️ Optional refactor for consistency
- const contentText = (() => { - if (isPending || isError) return null; - if (typeof output === "string" && output.trim()) return output.trim(); - if (outputObject) { - const c = outputObject.content ?? outputObject.text; - if (typeof c === "string" && c.trim()) return c.trim(); - } - return null; - })(); + const contentText = useMemo(() => { + if (isPending || isError) return null; + if (typeof output === "string" && output.trim()) return output.trim(); + if (outputObject) { + const c = outputObject.content ?? outputObject.text; + if (typeof c === "string" && c.trim()) return c.trim(); + } + return null; + }, [isPending, isError, output, outputObject]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/SupersetToolCall/SupersetToolCall.tsx` around lines 55 - 63, Replace the IIFE that computes contentText in SupersetToolCall with a useMemo hook to mirror errorText's pattern: compute contentText based on isPending, isError, output, and outputObject inside useMemo and include the same trimming/null logic (check typeof output === "string" && output.trim(), fall back to outputObject.content ?? outputObject.text, trim) so it only recalculates when its dependencies (isPending, isError, output, outputObject) change.apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/SlashCommandPreviewPopover.tsx (1)
37-55: TheinputValuedependency is intentional for re-triggering layout.The static analysis flags
inputValueas unnecessary, but it serves as a trigger to recompute the anchor position when the editor content changes (which may move the slash-command chip). The effect reads fromeditor.state.docwhich implicitly depends on input changes. This is a valid pattern, though adding a comment would clarify the intent.📝 Add clarifying comment
// Position the virtual anchor over the slash-command chip in the editor. +// inputValue is included in deps to re-trigger when content changes move the chip. useLayoutEffect(() => { const el = anchorRef.current; if (!el) return;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/SlashCommandPreviewPopover.tsx` around lines 37 - 55, The effect in useLayoutEffect that computes the anchor position for SlashCommandPreviewPopover intentionally includes inputValue as a dependency to re-run when editor content moves the slash-command chip; update the block around useLayoutEffect to add a concise clarifying comment explaining that inputValue is used solely to trigger recomputation of the anchor (while the effect reads editor.state.doc and uses anchorRef, foundPos, and editor.view.nodeDOM), so future reviewers won't remove the dependency.apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/TiptapPromptEditor.tsx (1)
27-29: Import statement placed after variable declarations.The import at line 29 appears after the
constdeclarations at lines 27-28. While this may work at runtime due to hoisting, it violates standard module organization conventions and could confuse readers.♻️ Move import to top with other imports
import Suggestion from "@tiptap/suggestion"; +import { useEffect, useLayoutEffect, useRef, useState } from "react"; +import { useDebouncedValue } from "renderer/hooks/useDebouncedValue"; +import { FileIcon } from "renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/utils"; const slashSuggestionKey = new PluginKey("slashCommandSuggestion"); const mentionSuggestionKey = new PluginKey("fileMentionSuggestion"); -import { useEffect, useLayoutEffect, useRef, useState } from "react"; -import { useDebouncedValue } from "renderer/hooks/useDebouncedValue"; -import { FileIcon } from "renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/utils";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/TiptapPromptEditor.tsx` around lines 27 - 29, Move the import statement (the React hooks import) up so all imports are grouped at the top of the module before any executable declarations; specifically ensure the PluginKey/related imports and the React import sit above the const declarations for slashSuggestionKey and mentionSuggestionKey (which reference PluginKey) so module organization is consistent and readers won't see variable declarations preceding imports.apps/desktop/src/renderer/components/Chat/components/SubagentInnerToolCall/SubagentInnerToolCall.tsx (1)
247-285: Redundant null check after already checkingparsedReadFileis truthy.The outer condition at line 250 already ensures
parsedReadFileis truthy. The innerif (parsed)check at line 253 is redundant sinceparsedis assigned fromparsedReadFile.♻️ Simplify by removing redundant check
if ( normalized === "mastra_workspace_read_file" && hasResult && parsedReadFile ) { - const parsed = parsedReadFile; - if (parsed) { - const filename = parsed.filename.split("/").pop() ?? parsed.filename; - const lineRange = `1–${parsed.lineCount}`; + const filename = parsedReadFile.filename.split("/").pop() ?? parsedReadFile.filename; + const lineRange = `1–${parsedReadFile.lineCount}`; // ... rest of the code using parsedReadFile directly - } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/Chat/components/SubagentInnerToolCall/SubagentInnerToolCall.tsx` around lines 247 - 285, The inner null-check is redundant: in SubagentInnerToolCall the outer guard already ensures parsedReadFile is truthy, so remove the nested "if (parsed)" block and use parsedReadFile (or the local const parsed = parsedReadFile) directly when rendering the ReadFileTool; update or remove the const parsed declaration accordingly and keep the rest of the logic (filename, lineRange, openInPane, detectLanguage, props) unchanged so behavior is identical but without the extra conditional.
🤖 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/components/Chat/ChatInterface/components/ChatInputFooter/components/QuestionInputOverlay/QuestionInputOverlay.tsx`:
- Around line 31-34: The reset effect currently runs only on mount (useEffect
with []), so submittedLabel and customText don't reset when a new question is
provided; update the useEffect in QuestionInputOverlay to depend on the incoming
question (or a stable question identifier) and call setSubmittedLabel(null) and
setCustomText("") whenever question changes, ensuring you reference the existing
setters (setSubmittedLabel, setCustomText) and the question prop/field used by
the component.
- Around line 44-51: The handler handleSubmitAnswer captures question.questionId
at creation time which can send a stale id if the question prop changes; update
the function to read const qid = question.questionId at call time (or store
questionId in a ref) and pass qid into onRespond(qid, answer) instead of using
the closed-over value, and improve error handling in the .catch by surfacing
feedback (e.g., call a toast/notification utility or set an inline error state)
while still resetting setSubmittedLabel(null) and setCustomText(""); keep
existing isDisabled short-circuiting behavior.
In
`@apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/SlashCommandNode.tsx`:
- Around line 8-9: The file references React.ChangeEvent and React.KeyboardEvent
but doesn’t import the React namespace; add a type-only import so TypeScript can
resolve those event types (e.g., add "import type { ChangeEvent, KeyboardEvent }
from 'react';" at the top) and update usages in SlashCommandNode (or any
handlers that use React.ChangeEvent/React.KeyboardEvent) to use ChangeEvent and
KeyboardEvent to avoid out-of-scope type errors under jsx: "react-jsx".
In
`@apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/TiptapPromptEditor.tsx`:
- Around line 570-587: The parseTextToEditorContent function currently only
converts `@mention` patterns and leaves "/{name}" slash command text as plain
text; update parseTextToEditorContent to detect the /^\/(\w+)(?:\s+(.*))?$/
pattern (or equivalent tokenization) and return a node of type "slash-command"
with attrs { name: capturedName, args: capturedArgs } (matching the editor
schema used by the SlashCommand node) so that when the useEffect in
TiptapPromptEditor uses editor.commands.setContent(...) the serialized slash
command text from controller.textInput.value is restored back into the editor as
a slash-command node rather than plain text; ensure the node shape and
attributes match the existing slash-command node implementation so selection and
further param updates continue to work.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/ChatMessageList/utils/messageListHelpers.ts:
- Line 1: The workspace chat path is currently dropping assistant messages that
contain an unanswered ask_user / ask_user_question tool call; update the
filtering in messageListHelpers.ts so assistant messages with tool calls of type
"ask_user" or "ask_user_question" are retained while the question is still
unanswered. Concretely, locate the filters that call hasAnsweredQuestionToolCall
(and any branches that drop assistant messages, e.g., in the functions
referenced around the earlier ranges) and change the logic to only exclude those
assistant messages if hasAnsweredQuestionToolCall(...) returns true; if it
returns false (question still pending) do not drop the message so the “Awaiting
Response” row and interrupted-message dedup behave the same as the tabbed chat
helper.
In `@packages/ui/src/components/ai-elements/clickable-file-path.tsx`:
- Around line 43-47: The onKeyDown handler in clickable-file-path.tsx for the
custom role="button" control needs to prevent the default Space behavior to
avoid scrolling; update the onKeyDown callback (the function that checks e.key
and calls onOpen) to call e.preventDefault() when the key is Space (e.g., e.key
=== " " or "Spacebar"/"Space") before e.stopPropagation() and onOpen(), so
keyboard activation triggers only the open action without scrolling the page.
In `@packages/ui/src/components/ai-elements/tool-call-row.tsx`:
- Around line 81-90: The error marker is only rendered in resolvedDescription
and is hidden when the row is expanded (isOpen), so expanded rows lose their
failure state; update the component (ToolCallRow) to compute a single
resolvedStatus/resolvedIcon (or reuse defaultStatus) that incorporates the
isError fallback (the red XCircle + "Error" marker) and render that status/icon
in both collapsed and expanded layouts instead of placing the fallback solely
inside resolvedDescription; ensure any other similar fallback instance around
lines that reference resolvedDescription (also the block around the second
occurrence noted) is replaced to use this shared resolvedStatus so the error
state stays visible regardless of isOpen.
- Around line 29-31: The header currently renders titleContent (interactive
ReactNodes) inside the toggle <button> and shows the error label only when
collapsed, causing nested focusable controls and disappearing error state; move
any interactive content passed via the title/titleContent out of the toggle
button and into the non-collapsible headerExtra slot (or render it alongside the
button) so the button only contains the toggle affordance and label text, and
change the error indicator rendering (statusNode / error label) so it is placed
in the header area independent of isOpen (render it beside the toggle or in
headerExtra) to keep the visual error state visible when expanded and avoid
nested focusable elements.
---
Outside diff comments:
In `@packages/ui/src/components/ai-elements/bash-tool.tsx`:
- Around line 43-44: The current error flag only checks exitCode, so rows with
state "output-error" are not treated as errors; update the derivation of isError
(used alongside isPending) to also consider state === "output-error" or
otherwise check BashToolState for the "output-error" branch so that any
output-error state is treated as an error even when exitCode is undefined
(referring to the isError variable and the state/exitCode values in
bash-tool.tsx).
---
Duplicate comments:
In
`@apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/parseTextToEditorContent.ts`:
- Around line 26-35: The loop uses an assignment inside the while condition
which trips the lint rule; refactor the loop so you call MENTION_RE.exec(line)
into the local variable match before checking it and break when null.
Concretely, in the function that contains MENTION_RE, replace the while ((match
= MENTION_RE.exec(line)) !== null) pattern with an explicit loop that sets match
= MENTION_RE.exec(line) at the top (or in a for(;;) loop) and breaks when match
=== null, then keep the existing body that pushes text nodes and the
file-mention node to inlineNodes and updates lastIndex (referencing MENTION_RE,
match, inlineNodes, lastIndex, and the file-mention attrs).
- Around line 9-45: The parser parseTextToEditorContent currently uses
MENTION_RE = /@(\S+)/g which incorrectly captures emails and leaves braces in
attrs.path and ignores slash-command chips; update the parsing logic in
parseTextToEditorContent to (1) detect the exact wire token forms produced by
serializeEditorToText: file mentions in the form @{path} (capture path without
braces) and standalone `@path` tokens only when they match the serializer's
unambiguous pattern (so do not treat email-like tokens as mentions), and (2)
detect slash-command chips serialized as "/{name} {args}" and emit a node (e.g.,
type "slash-command" with attrs name and args). Replace MENTION_RE with a regex
that recognizes the @{...} form (and only the allowed unbraced form per
serializer rules) and add a SLASH_RE for "/{name} {args}", then update the
while/loop in parseTextToEditorContent to branch on which capture matched and
push the appropriate { type: "file-mention", attrs: { path } } or { type:
"slash-command", attrs: { name, args } } nodes, leaving ordinary text (e.g.,
foo@bar.com) untouched.
In
`@apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/AskUserQuestionToolCall/AskUserQuestionToolCall.tsx`:
- Around line 231-251: The useMemo hook (creating answeredQAs) is called after
an early return which violates the Rules of Hooks; move the useMemo block that
computes answeredQAs (using useMemo, answeredQAs, questions, answers, and
findAnswerForQuestion) above the early return that checks questions.length,
isCancelledByError, and isCancelledByStop so hooks are executed unconditionally
and maintain stable order across renders; keep the same dependencies
([questions, answers]) and preserve the filtering logic and type guard.
In
`@apps/desktop/src/renderer/components/Chat/components/SubagentInnerToolCall/SubagentInnerToolCall.tsx`:
- Around line 217-246: The useMemo call (creating parsedReadFile via
parseReadFileResult) is placed after an early return for the
mastra_workspace_execute_command branch, causing a hooks order violation when
normalized changes; move the useMemo block above the conditional that returns
the BashTool so all hooks run unconditionally in the same order (keep the same
dependency array [normalized, hasResult, result] and logic checking normalized
=== "mastra_workspace_read_file" && hasResult), then retain the existing early
return that renders BashTool for normalized ===
"mastra_workspace_execute_command".
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPaneInterface/ChatPaneInterface.tsx`:
- Around line 981-1002: Before optimistically flipping state in
handleQuestionResponse, capture the previous pane status and the previous
answeredQuestionId, then wrap the respondToQuestion call in try/catch; on
rejection restore answeredQuestionId (via setAnsweredQuestionId) and restore the
pane status (via useTabsStore.getState().setPaneStatus(paneId, previousStatus)
or at minimum set it back to the pending/question status), and still clear
setQuestionResponsePending in finally; reference handleQuestionResponse,
answeredQuestionId/setAnsweredQuestionId, useTabsStore.getState().setPaneStatus,
and commands.respondToQuestion when making the change.
In `@packages/host-service/src/runtime/chat/chat.ts`:
- Around line 354-380: The ensureGlobalAgentInstructions method currently writes
Superset-specific rules into a user-global file (AGENTS.md) which must be
removed; stop mutating ~/.mastracode/AGENTS.md by deleting or turning
ensureGlobalAgentInstructions into a no-op and remove uses of
MANAGED_MARKER/INSTRUCTIONS that perform filesystem writes, and instead inject
the same instruction text (INSTRUCTIONS) into the runtime/session-level
configuration used when creating the chat runtime or agent (e.g., include the
instruction in the system prompt or session-specific agent instructions where
the chat/session is constructed), so the ask_user policy is enforced per-session
without altering global user files.
In `@packages/ui/src/components/ai-elements/code-block.tsx`:
- Around line 67-88: The catch currently retries by calling highlightCode with
"text" for every exception which can recurse if the failure is from shiki/theme
init; modify the try/catch in code-block.tsx around codeToHtml so it
distinguishes unsupported-language errors from other failures or adds a one-time
retry guard (e.g., a boolean retryAttempted local flag or parameter) before
falling back to highlightCode; specifically update the block that calls
codeToHtml and the fallback call to highlightCode so that if the fallback itself
fails you surface or rethrow the original error instead of recursively retrying.
In `@packages/ui/src/components/ai-elements/show-code.tsx`:
- Around line 126-200: The icon-only Buttons in the ShowCode component are
missing accessible names; add aria-label attributes to each Button: for the
expand/collapse toggle Button (used with setIsExpanded and isExpanded when
isOverflowing) set aria-label to either "Collapse" or "Expand" based on
isExpanded; for the external open Button (onClick calling onOpen when filename
is present) set aria-label to "Open" or include the filename like `Open
${filename}`; and for the copy Button (onClick -> handleCopy) set aria-label to
"Copy code" (or similar). Ensure you update the Button props where those Buttons
are rendered so screen readers receive a clear label.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/SlashCommandPreviewPopover.tsx`:
- Around line 37-55: The effect in useLayoutEffect that computes the anchor
position for SlashCommandPreviewPopover intentionally includes inputValue as a
dependency to re-run when editor content moves the slash-command chip; update
the block around useLayoutEffect to add a concise clarifying comment explaining
that inputValue is used solely to trigger recomputation of the anchor (while the
effect reads editor.state.doc and uses anchorRef, foundPos, and
editor.view.nodeDOM), so future reviewers won't remove the dependency.
In
`@apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/TiptapPromptEditor.tsx`:
- Around line 27-29: Move the import statement (the React hooks import) up so
all imports are grouped at the top of the module before any executable
declarations; specifically ensure the PluginKey/related imports and the React
import sit above the const declarations for slashSuggestionKey and
mentionSuggestionKey (which reference PluginKey) so module organization is
consistent and readers won't see variable declarations preceding imports.
In
`@apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/SupersetToolCall/SupersetToolCall.tsx`:
- Around line 55-63: Replace the IIFE that computes contentText in
SupersetToolCall with a useMemo hook to mirror errorText's pattern: compute
contentText based on isPending, isError, output, and outputObject inside useMemo
and include the same trimming/null logic (check typeof output === "string" &&
output.trim(), fall back to outputObject.content ?? outputObject.text, trim) so
it only recalculates when its dependencies (isPending, isError, output,
outputObject) change.
In
`@apps/desktop/src/renderer/components/Chat/components/SubagentInnerToolCall/SubagentInnerToolCall.tsx`:
- Around line 247-285: The inner null-check is redundant: in
SubagentInnerToolCall the outer guard already ensures parsedReadFile is truthy,
so remove the nested "if (parsed)" block and use parsedReadFile (or the local
const parsed = parsedReadFile) directly when rendering the ReadFileTool; update
or remove the const parsed declaration accordingly and keep the rest of the
logic (filename, lineRange, openInPane, detectLanguage, props) unchanged so
behavior is identical but without the extra conditional.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/ChatPaneInterface.tsx:
- Around line 534-553: The current extraction using /\/(\S+)/g (producing
contentWithSkillsExtracted, skillNames and using findSlashCommandByNameOrAlias
against slashCommands) strips leading slashes even when users are merely
mentioning a command, causing false positives; change the extraction to only
recognize explicit chip/token syntax from the editor (e.g., Tiptap node marks or
a serialized chip token) rather than a generic "/word" regex—update the logic in
the component that builds contentWithSkillsExtracted to query the
editor/serialized node tree or match the editor's chip marker (or require a
preceding whitespace/start-of-line and trailing boundary) and only then push to
skillNames, leaving all other raw text unchanged so resolveSlashCommandInput
still handles builtin commands correctly.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPaneInterface/components/ChatMessageList/utils/messageListHelpers.ts`:
- Around line 107-112: The duplicated predicate that decides if an assistant
message with an ask_user tool call should be visible (currently written as
message.role !== "assistant" || hasAnsweredQuestionToolCall(message) ||
hasPendingQuestionToolCall(message)) is repeated in this file (and similar logic
at lines 162-167); extract that logic into a single helper (e.g.,
isAskUserVisibleForMessage or isActiveTurnQuestionVisible) and replace both
inline uses with a call to that helper, referencing the existing helpers
hasAnsweredQuestionToolCall and hasPendingQuestionToolCall inside it so
visibility rules are centralized.
In `@packages/ui/src/components/ai-elements/file-diff-tool.tsx`:
- Around line 123-130: Remove the dead state and effect: delete the useState
declaration for hasAutoExpanded and its setter (setHasAutoExpanded) and remove
the useEffect that checks hasAutoExpanded and hasExpandedRenderer; the only
remaining signal is hasExpandedRenderer (derived from renderExpandedContent) so
eliminating hasAutoExpanded/useEffect cleans up unused state and side-effect
logic in file-diff-tool.tsx.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
michalkopanski
had a problem deploying
to
preview
GitHub Actions
Failure
Contributor
There was a problem hiding this comment.
4 issues found across 84 files
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed. cubic prioritises the most important files to review.
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/components/Chat/ChatInterface/components/TiptapPromptEditor/FileMentionNode.tsx">
<violation number="1" location="apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/FileMentionNode.tsx:10">
P2: File mention node view assumes `path` is a string even though the schema allows `null`, so malformed or pasted content can crash rendering.</violation>
</file>
<file name="apps/desktop/src/renderer/components/Chat/components/SubagentInnerToolCall/SubagentInnerToolCall.tsx">
<violation number="1" location="apps/desktop/src/renderer/components/Chat/components/SubagentInnerToolCall/SubagentInnerToolCall.tsx:151">
P2: Uses slash-only path parsing for display names, so Windows paths are not reduced to filenames and the chat tool UI shows full raw paths.</violation>
</file>
<file name="apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/parseTextToEditorContent.ts">
<violation number="1" location="apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/parseTextToEditorContent.ts:8">
P2: File mention paths containing spaces cannot round-trip through the new text parser because it tokenizes mentions with `\S+`.</violation>
</file>
<file name="apps/desktop/src/renderer/components/Chat/ChatInterface/components/ReadOnlyToolCall/ReadOnlyToolCall.tsx">
<violation number="1" location="apps/desktop/src/renderer/components/Chat/ChatInterface/components/ReadOnlyToolCall/ReadOnlyToolCall.tsx:91">
P2: Line-range labels are offset from tool args even though the renderer rereads the whole file, so the visible line numbers can be wrong for read_file outputs.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/AskUserQuestionToolCall/AskUserQuestionToolCall.tsx (1)
30-36:⚠️ Potential issue | 🟡 MinorDead props:
isStreamingandonAnswerare declared in the interface but never used in the component.The function signature (line 172) destructures only
part,args,result,outputObject,nestedResultObject, andisInterrupted—it does not destructureisStreaming(line 30) oronAnswer(lines 32-35), and these props are never referenced anywhere in the component body. Meanwhile,ToolCallBlock.tsxcontinues to forward both props to this component (lines 503-505).Since
AskUserQuestionToolCallis the only consumer ofisStreamingandonAnswerfromToolCallBlockProps, removing them from both the component interface and the call site is safe and will improve clarity.♻️ Proposed cleanup
Remove from
AskUserQuestionToolCallProps:interface AskUserQuestionToolCallProps { part: ToolPart; args: Record<string, unknown>; result: Record<string, unknown>; outputObject?: Record<string, unknown>; nestedResultObject?: Record<string, unknown>; - isStreaming?: boolean; isInterrupted?: boolean; - onAnswer?: ( - toolCallId: string, - answers: Record<string, string>, - ) => Promise<void> | void; }Then remove the corresponding forwarding in
ToolCallBlock.tsx(around lines 503–505).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/AskUserQuestionToolCall/AskUserQuestionToolCall.tsx` around lines 30 - 36, Remove the unused props isStreaming and onAnswer from the AskUserQuestionToolCallProps interface and from the AskUserQuestionToolCall component (the function that currently destructures part, args, result, outputObject, nestedResultObject, isInterrupted); then update the caller ToolCallBlock to stop forwarding isStreaming and onAnswer to AskUserQuestionToolCall (remove them where ToolCallBlock spreads/passes props). Make sure to update any related type references or imports so the AskUserQuestionToolCall signature and ToolCallBlock prop forwarding remain consistent.
♻️ Duplicate comments (1)
apps/desktop/src/renderer/components/Chat/ChatInterface/components/ReadOnlyToolCall/ReadOnlyToolCall.tsx (1)
79-86:⚠️ Potential issue | 🟠 Major
staleTime: Infinitystill prevents picking up on-disk changes.The query key is just
workspaceId+absolutePath+encoding, so if the same file is read again after it has been modified on disk (e.g., a laterread_filetool call, or the user edits then re-opens the chat), TanStack Query will happily serve the originally cached contents and never refetch. Per the PR description ("ReadOnlyToolCall reads current disk content"), this query should treat data as immediately stale. SettingstaleTime: 0is sufficient — v5 will refetch on mount when stale, andrefetchOnWindowFocus: false/retry: falsealready cover the other axes.Proposed fix
{ enabled: isReadFileTool && !isPending && !!absoluteFilePath && !!workspaceId, retry: false, refetchOnWindowFocus: false, - staleTime: Infinity, + staleTime: 0, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/Chat/ChatInterface/components/ReadOnlyToolCall/ReadOnlyToolCall.tsx` around lines 79 - 86, The query options in ReadOnlyToolCall's useQuery call currently set staleTime: Infinity which prevents refetching updated on-disk file contents; change the option to staleTime: 0 in the options object (the same block that contains enabled, retry, refetchOnWindowFocus) so TanStack Query treats the file data as immediately stale and will refetch on mount when the file changes (keep retry: false and refetchOnWindowFocus: false as-is).
🧹 Nitpick comments (3)
packages/ui/src/components/ai-elements/show-code.tsx (1)
174-180: ConsiderstopPropagationon the copy button click.The Open button stops propagation (line 158) to prevent ancestor click handlers (e.g., row expansion in
ToolCallRow) from firing. The copy button doesn't, so clicking copy may also toggle the surrounding tool-call row. Worth aligning for consistency.🔧 Suggested change
- const handleCopy = async () => { + const handleCopy = async (e: React.MouseEvent) => { + e.stopPropagation(); try { await navigator.clipboard.writeText(code);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ai-elements/show-code.tsx` around lines 174 - 180, The copy Button in show-code.tsx can trigger ancestor click handlers (e.g., ToolCallRow expansion); update the copy click handler to stop event propagation: modify the onClick (or the handleCopy function) to accept the click event and call event.stopPropagation() before performing the copy logic so the copy button behaves like the Open button which already stops propagation.apps/desktop/src/renderer/components/Chat/components/SubagentInnerToolCall/SubagentInnerToolCall.tsx (1)
247-285: Redundant nested null check onparsedReadFile.
parsedReadFileis already asserted truthy in the outerifat line 250, so the innerif (parsed)at line 253 is dead code. Flatten the branch.♻️ Proposed simplification
- if ( - normalized === "mastra_workspace_read_file" && - hasResult && - parsedReadFile - ) { - const parsed = parsedReadFile; - if (parsed) { - const filename = parsed.filename.split("/").pop() ?? parsed.filename; - const lineRange = `1–${parsed.lineCount}`; - ... - return ( - <ReadFileTool ... /> - ); - } - } + if ( + normalized === "mastra_workspace_read_file" && + hasResult && + parsedReadFile + ) { + const filename = + parsedReadFile.filename.split("/").pop() ?? parsedReadFile.filename; + const lineRange = `1–${parsedReadFile.lineCount}`; + ... + return ( + <ReadFileTool ... /> + ); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/Chat/components/SubagentInnerToolCall/SubagentInnerToolCall.tsx` around lines 247 - 285, The code contains a redundant nested null check: parsedReadFile is already checked truthy in the outer if (normalized === "mastra_workspace_read_file" && hasResult && parsedReadFile), so remove the inner if (parsed) block and use parsedReadFile directly (e.g., replace const parsed = parsedReadFile; and the inner guard with direct usage) when constructing filename, lineRange, openInPane and the ReadFileTool props in the SubagentInnerToolCall component; ensure onOpenFileInPane, normalizeWorkspaceFilePath, detectLanguage and the existing variables (args, workspaceCwd, isError, isPending) are used unchanged.apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/ChatPaneInterface.tsx (1)
535-554: Simplify the skill-extraction branch and reconsider side-effects in.replace.A few small observations:
contentWithSkillsExtractedalready equalscontentwhen no custom commands match (every pass-through returnsmatch), so theif (skillNames.length > 0)guard around the assignment is effectively redundant — you can unconditionally assign (or keep the guard purely as an optimization, in which case a comment noting that would help).- Mutating
skillNamesinside aString.prototype.replacecallback works but mixes pure string transformation with collection-building. A pre-scan viamatchAllfollowed by a singlereplacepass (or a plain loop) reads a bit more cleanly, though this is a stylistic nit.- Greedy
\S+plus the pass-through-on-unknown behavior correctly keeps paths/URLs (e.g./usr/local/bin,https://…) out ofskillNames, so no correctness issue there.Optional refactor
- const skillNames: string[] = []; - const contentWithSkillsExtracted = content.replace( - /\/(\S+)/g, - (match, name: string) => { - const command = findSlashCommandByNameOrAlias(slashCommands, name); - if (command?.kind === "custom") { - if (!skillNames.includes(command.name)) { - skillNames.push(command.name); - } - return name; // strip the leading / - } - return match; - }, - ); - if (skillNames.length > 0) { - content = contentWithSkillsExtracted.trim(); - } + const skillNameSet = new Set<string>(); + content = content + .replace(/\/(\S+)/g, (match, name: string) => { + const command = findSlashCommandByNameOrAlias(slashCommands, name); + if (command?.kind !== "custom") return match; + skillNameSet.add(command.name); + return name; // strip the leading / + }) + .trim(); + const skillNames = Array.from(skillNameSet);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/ChatPaneInterface.tsx around lines 535 - 554, The skill extraction mixes side-effects into the replace callback and conditionally assigns content only when skillNames is non-empty; refactor to first scan for slash tokens (using matchAll or a loop) and populate skillNames via findSlashCommandByNameOrAlias(slashCommands, name), then perform a single deterministic replace to strip leading slashes from custom commands and always assign the result to content (replace the current contentWithSkillsExtracted logic and remove the conditional assignment), which keeps mutation separate from pure string transformation and simplifies the branch around contentWithSkillsExtracted/skillNames.
🤖 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/components/Chat/ChatInterface/components/ReadOnlyToolCall/ReadOnlyToolCall.tsx`:
- Around line 91-103: The current lineRange calculation (variables lineRange,
startLine, endLine, args, fileContent) wrongly computes endLine as startLine +
lineCount - 1 and leaves a dead ternary; fix by first computing total lines =
fileContent.split("\n").length, compute startLine from args (args.startLine ||
args.start_line || args.offset || args.from || 1), then determine endLine by
honoring explicit args.endLine/args.end_line or a provided args.limit (e.g., end
= startLine + limit - 1 clamped to total lines) and if no end/limit is provided
assume the whole file was returned so use endLine = totalLines and set startLine
= 1 when appropriate; finally set lineRange to `${startLine}–${endLine}` (no
dead ternary) so it reflects requested slice or the actual full-file range.
- Line 187: The unsafe cast at language={detectLanguage(displayPath) as
BundledLanguage} is due to detectLanguage() returning "plaintext" as a fallback
which Shiki doesn't accept; update detectLanguage to return "text" (Shiki's
valid fallback) instead of "plaintext" so the value is a valid BundledLanguage
and the cast/exception retry in highlightCode is no longer needed; search for
and change the fallback literal in the detectLanguage function and ensure
callers like ReadOnlyToolCall still type-check against BundledLanguage.
In
`@apps/desktop/src/renderer/components/Chat/components/SubagentInnerToolCall/SubagentInnerToolCall.tsx`:
- Around line 254-255: The computed lineRange currently hardcodes the start as
1; instead extract the true start line from the args object (check
args.startLine, args.start_line, args.offset, args.from in that fallback order)
and compute startLineNumber = parsed.startLine ?? the extracted value ?? 1, then
set lineRange = `${startLineNumber}–${startLineNumber + parsed.lineCount - 1}`
so the displayed range reflects the tool's offset; update the logic in
SubagentInnerToolCall.tsx where filename and lineRange are computed (use the
same fallback chain as ReadOnlyToolCall's startLine resolution).
In `@packages/ui/src/components/ai-elements/code-block.tsx`:
- Around line 80-91: The catch block in the codeToHtml path currently returns
["", ""] when language === "text", which yields an empty code block; instead,
ensure the original code is rendered as a safe plain-text fallback by calling
highlightCode(code, "text" as BundledLanguage, showLineNumbers, startLine) (or
otherwise returning an escaped <pre> of the code) rather than returning an empty
string; update the catch branch in code-block.tsx to remove the ["",""] return
and invoke highlightCode with the existing variables (language, code,
showLineNumbers, startLine) so text input remains visible when Shiki fails.
In `@packages/ui/src/components/ai-elements/show-code.tsx`:
- Around line 83-84: Trim a single trailing newline from the input before
computing lineCount so files ending with "\n" don't artificially increment the
count: modify the logic that computes lineCount/isOverflowing (variables `code`,
`lineCount`, `isOverflowing`, and `maxLines`) to strip one trailing "\n" (not
full whitespace) then split on "\n" to get an accurate line count and recompute
`isOverflowing`.
---
Outside diff comments:
In
`@apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/AskUserQuestionToolCall/AskUserQuestionToolCall.tsx`:
- Around line 30-36: Remove the unused props isStreaming and onAnswer from the
AskUserQuestionToolCallProps interface and from the AskUserQuestionToolCall
component (the function that currently destructures part, args, result,
outputObject, nestedResultObject, isInterrupted); then update the caller
ToolCallBlock to stop forwarding isStreaming and onAnswer to
AskUserQuestionToolCall (remove them where ToolCallBlock spreads/passes props).
Make sure to update any related type references or imports so the
AskUserQuestionToolCall signature and ToolCallBlock prop forwarding remain
consistent.
---
Duplicate comments:
In
`@apps/desktop/src/renderer/components/Chat/ChatInterface/components/ReadOnlyToolCall/ReadOnlyToolCall.tsx`:
- Around line 79-86: The query options in ReadOnlyToolCall's useQuery call
currently set staleTime: Infinity which prevents refetching updated on-disk file
contents; change the option to staleTime: 0 in the options object (the same
block that contains enabled, retry, refetchOnWindowFocus) so TanStack Query
treats the file data as immediately stale and will refetch on mount when the
file changes (keep retry: false and refetchOnWindowFocus: false as-is).
---
Nitpick comments:
In
`@apps/desktop/src/renderer/components/Chat/components/SubagentInnerToolCall/SubagentInnerToolCall.tsx`:
- Around line 247-285: The code contains a redundant nested null check:
parsedReadFile is already checked truthy in the outer if (normalized ===
"mastra_workspace_read_file" && hasResult && parsedReadFile), so remove the
inner if (parsed) block and use parsedReadFile directly (e.g., replace const
parsed = parsedReadFile; and the inner guard with direct usage) when
constructing filename, lineRange, openInPane and the ReadFileTool props in the
SubagentInnerToolCall component; ensure onOpenFileInPane,
normalizeWorkspaceFilePath, detectLanguage and the existing variables (args,
workspaceCwd, isError, isPending) are used unchanged.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/ChatPaneInterface.tsx:
- Around line 535-554: The skill extraction mixes side-effects into the replace
callback and conditionally assigns content only when skillNames is non-empty;
refactor to first scan for slash tokens (using matchAll or a loop) and populate
skillNames via findSlashCommandByNameOrAlias(slashCommands, name), then perform
a single deterministic replace to strip leading slashes from custom commands and
always assign the result to content (replace the current
contentWithSkillsExtracted logic and remove the conditional assignment), which
keeps mutation separate from pure string transformation and simplifies the
branch around contentWithSkillsExtracted/skillNames.
In `@packages/ui/src/components/ai-elements/show-code.tsx`:
- Around line 174-180: The copy Button in show-code.tsx can trigger ancestor
click handlers (e.g., ToolCallRow expansion); update the copy click handler to
stop event propagation: modify the onClick (or the handleCopy function) to
accept the click event and call event.stopPropagation() before performing the
copy logic so the copy button behaves like the Open button which already stops
propagation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: eb552dc9-4bd8-45fe-a6e7-1359bf9e72a6
📒 Files selected for processing (10)
apps/desktop/src/renderer/components/Chat/ChatInterface/components/ReadOnlyToolCall/ReadOnlyToolCall.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/parseTextToEditorContent.tsapps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/components/AskUserQuestionToolCall/AskUserQuestionToolCall.tsxapps/desktop/src/renderer/components/Chat/components/SubagentInnerToolCall/SubagentInnerToolCall.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/ChatPaneInterface.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/ChatMessageList/ChatMessageList.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPaneInterface/ChatPaneInterface.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPaneInterface/components/ChatMessageList/ChatMessageList.tsxpackages/ui/src/components/ai-elements/code-block.tsxpackages/ui/src/components/ai-elements/show-code.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/parseTextToEditorContent.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPaneInterface/components/ChatMessageList/ChatMessageList.tsx
Contributor
There was a problem hiding this comment.
1 issue found across 17 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/components/Chat/ChatInterface/components/TiptapPromptEditor/TiptapPromptEditor.tsx">
<violation number="1" location="apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/TiptapPromptEditor.tsx:480">
P2: Enter/Tab is no longer consumed when the mention list is empty or the selected index is invalid, so the key can fall through to default editor/browser behavior while the mention popup is still open.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/ToolCallBlock.tsx (1)
456-456: Redundant equality check.
toolName === "web_search"is already covered bytoolName.includes("web_search"), so the first clause is dead.♻️ Suggested simplification
- if (toolName === "web_search" || toolName.includes("web_search")) { + if (toolName.includes("web_search")) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/ToolCallBlock.tsx` at line 456, The conditional in ToolCallBlock.tsx redundantly checks toolName === "web_search" and toolName.includes("web_search"); remove the dead equality check and simplify the if to only use toolName.includes("web_search") (or another single check like startsWith if that better matches intent) where the variable toolName is evaluated in the ToolCallBlock component.packages/ui/src/components/ai-elements/code-block.tsx (1)
138-155: Standardize on Tailwind v4 suffix!syntax for consistency.The code mixes suffix form (
[&>pre]:bg-background!,[&>pre]:text-foreground!) with prefix form ([&_span[style]]:!text-foreground,[&_.line>.shiki-line-number]:!opacity-50). Tailwind v4 standard places the!at the end of the class name; the prefix form is deprecated v3 syntax. Standardize all arbitrary selector modifiers to suffix form:[&_span[style]]:text-foreground!and[&_.line>.shiki-line-number]:opacity-50!.The
[&_span[style]]:text-foreground!approach is correct—it relies on Shiki's default inlinestyle="color:…"emission to override colorized tokens whencolorize={false}.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ai-elements/code-block.tsx` around lines 138 - 155, Update the Tailwind arbitrary modifier suffixes in the CodeBlock JSX where html and darkHtml are rendered: replace prefix-style usages like "[&_span[style]]:!text-foreground" and "[&_.line>.shiki-line-number]:!opacity-50" with the v4 suffix form "[&_span[style]]:text-foreground!" and "[&_.line>.shiki-line-number]:opacity-50!" inside the className prop (the same change for both the light and dark divs that use colorize and dangerouslySetInnerHTML with html/darkHtml) so all selector overrides use the trailing "!" syntax consistently.
🤖 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/components/Chat/ChatInterface/components/TiptapPromptEditor/SlashCommandPreviewPopover.tsx`:
- Around line 37-55: The anchor positioning only runs once because
useLayoutEffect depends only on the stable editor; subscribe to editor updates
and viewport changes, factor the measurement into a measureAnchor function that
(1) traverses editor.state.doc.descendants to find the first node with
node.type.name === "slash-command" (guard at the top of the callback with a
found flag and return early so you don't keep overwriting foundPos), (2) map
foundPos to a DOM node via editor.view.nodeDOM(foundPos) and update
anchorRef.current.style.left/top/width/height, and (3) call measureAnchor from
an editor.on('update', measureAnchor) listener plus
window.addEventListener('scroll', measureAnchor, {passive:true}) and
window.addEventListener('resize', measureAnchor); register those listeners in
the effect and remove them in cleanup to avoid leaks.
In
`@apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/TiptapPromptEditor.tsx`:
- Around line 474-481: When Enter or Tab is pressed inside the mention popover,
the current code returns false when there are no files, letting the key fall
through; change the handler so it always consumes Enter and Tab while a mention
is active: keep the existing branch that uses files[mention.selectedIndex] and
calls mention.tiptapCommand({ path: file.relativePath }) when a file exists, but
when no file is found (files is empty or undefined) return true instead of false
so the key is swallowed (optionally also trigger the mention dismissal logic if
you have a method to close the popover). This targets the
mention.selectedIndex/files check and preserves behavior of
mention.tiptapCommand and mentionStateRef.current.
- Around line 290-303: The onUpdate handler for the slash menu (inside
TiptapPromptEditor's setSlashMenu callback) preserves prev.selectedIndex
verbatim which can exceed the new props.items length after filtering; update
that callback to clamp selectedIndex to the new commands array bounds (e.g., if
prev.selectedIndex is greater than props.items.length - 1, set it to
props.items.length - 1, and handle empty list by setting 0 or null) similar to
how the mention handler clamps selectedIndex when mentionFiles changes so
pressing Enter won't index past the end of menu.commands.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/ChatInputFooter/ChatInputFooter.tsx:
- Around line 147-155: The ChatInputFooter lacks the focus handoff when the
QuestionInputOverlay unmounts: detect when pendingQuestion transitions from a
non-null value to null and, inside a useEffect, call
usePromptInputController().textInput.focus() wrapped in requestAnimationFrame so
focus returns to the Tiptap editor; trigger this after onQuestionRespond and
onQuestionCancel flow (i.e., where QuestionInputOverlay is unmounted) to mirror
the behavior from ChatInputFooter.tsx and ensure the editor regains focus.
In `@packages/ui/src/components/ai-elements/clickable-file-path.tsx`:
- Around line 26-27: The label computation in clickable-file-path.tsx can yield
an empty string for paths ending with "/" because path.split("/").pop() returns
"" and the nullish coalescing (??) doesn't catch it; update the label assignment
(the const label = ... expression) to treat empty string as falsy so it falls
back to path (for example replace the nullish coalescing with a falsy check like
using || or explicitly check for empty string) so trailing-slash paths produce
the original path instead of an empty label.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/ToolCallBlock.tsx`:
- Line 456: The conditional in ToolCallBlock.tsx redundantly checks toolName ===
"web_search" and toolName.includes("web_search"); remove the dead equality check
and simplify the if to only use toolName.includes("web_search") (or another
single check like startsWith if that better matches intent) where the variable
toolName is evaluated in the ToolCallBlock component.
In `@packages/ui/src/components/ai-elements/code-block.tsx`:
- Around line 138-155: Update the Tailwind arbitrary modifier suffixes in the
CodeBlock JSX where html and darkHtml are rendered: replace prefix-style usages
like "[&_span[style]]:!text-foreground" and
"[&_.line>.shiki-line-number]:!opacity-50" with the v4 suffix form
"[&_span[style]]:text-foreground!" and
"[&_.line>.shiki-line-number]:opacity-50!" inside the className prop (the same
change for both the light and dark divs that use colorize and
dangerouslySetInnerHTML with html/darkHtml) so all selector overrides use the
trailing "!" syntax consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6910654d-0c5a-4c9b-8558-a9f42a843be5
📒 Files selected for processing (16)
apps/desktop/src/renderer/components/Chat/ChatInterface/components/ChatInputFooter/ChatInputFooter.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/ChatInputFooter/components/QuestionInputOverlay/QuestionInputOverlay.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/ReadOnlyToolCall/ReadOnlyToolCall.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/FileMentionNode.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/SlashCommandNode.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/SlashCommandPreviewPopover.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/TiptapPromptEditor.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/parseTextToEditorContent.tsapps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/serializeEditorToText.tsapps/desktop/src/renderer/components/Chat/ChatInterface/components/ToolCallBlock/ToolCallBlock.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/ChatInputFooter/ChatInputFooter.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/ChatMessageList/utils/messageListHelpers.tspackages/ui/src/components/ai-elements/clickable-file-path.tsxpackages/ui/src/components/ai-elements/code-block.tsxpackages/ui/src/components/ai-elements/show-code.tsxpackages/ui/src/components/ai-elements/tool-call-row.tsx
✅ Files skipped from review due to trivial changes (1)
- apps/desktop/src/renderer/components/Chat/ChatInterface/components/ReadOnlyToolCall/ReadOnlyToolCall.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/ChatMessageList/utils/messageListHelpers.ts
- apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/serializeEditorToText.ts
- apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/FileMentionNode.tsx
- apps/desktop/src/renderer/components/Chat/ChatInterface/components/ChatInputFooter/components/QuestionInputOverlay/QuestionInputOverlay.tsx
- packages/ui/src/components/ai-elements/tool-call-row.tsx
Remove SubagentExecutionMessage from the bottom-pinned section so subagent tool calls render inline within AssistantMessage via the existing SubagentToolCall collapsible component, consistent with all other tool calls.
Removes top padding from ChatInputDropZone so content is flush with the bottom of the scroll area, and adds a CSS pseudo-element gradient that fades the conversation content into the background above the input bar.
- ToolInput/ToolOutput: replace CodeBlock with plain <pre>, remove p-4 padding, switch to bg-muted/30 backgrounds, rename labels to "Input"/"Output", apply font-mono throughout - ToolHeader: reduce px-2.5 → px-1, add rounded-b-md, swap chevron direction on hover (right=collapsed, down=expanded), add open prop - Tool: add font-mono to outer Collapsible wrapper
Expanding a tool call was causing the scroll container to jump. Detect clicks on [data-tool-trigger] elements and call stopScroll() so the stick-to-bottom behaviour doesn't fight the user interaction.
Introduces a shared ToolCallRow component that encapsulates the common collapsible row pattern: icon/chevron, ShimmerLabel title, muted description, status slot, left-border content area, and an optional headerExtra element for out-of-trigger action buttons. Refactors BashTool, WebSearchTool, WebFetchTool, and FileDiffTool to use ToolCallRow, removing duplicated hover/chevron/collapsible logic from each. All tools now have consistent: font-mono, px-1 header padding, rounded-b-md on hover, ml-2.5 border-l content alignment, and chevron-right/down hover behaviour.
Migrates GenericToolCall, SupersetToolCall, SubagentToolCall, and ReadOnlyToolCall to use the new ToolCallRow component, eliminating per-component chevron/hover/collapsible boilerplate. Also updates ToolCallBlock dispatch: web_search tool variants without parsed results now fall through to GenericToolCall with a globe icon instead of WebSearchTool (which requires results to be expandable). Renames "Subagent" label to "Agent" with the agent type as a muted description.
- Add QuestionInputOverlay component: numbered options, "Something else" free-text row with pencil icon, Skip button, cross-fade to submit on type - Refactor AskUserQuestionToolCall to plain ToolCallRow (no inline answer UI) - Wire pendingQuestion/handleQuestionResponse/stopActiveResponse to footer in both ChatPane variants instead of ChatMessageList - Remove PendingQuestionMessage from ChatMessageList and clean up its props
…d left-side icons - Add BrailleSpinner component (braille chars, amber, matches sidenav style) - Show spinner in icon slot while pending, red X on error, icon on complete - Remove right-side status slot (no spinner, checkmark, or X on the right) - Remove title shimmer animation
…n after answered/interrupted - Thread isStreaming through ToolCallBlock to AskUserQuestionToolCall - Return null only when isPending && still streaming (overlay is active) - Show collapsed row with question text as description once answered or interrupted
- Align tool call icon with chat text using -mx-1 negative margin - Remove overflow-hidden from MessageContent to avoid clipping - Hide description in tool call header when expanded - Show query field as plain text with Query/Response labels instead of raw JSON - Add subtle focus-visible ring to collapsible trigger button - Adjust content padding (pl-3 py-1) to align with heading text - Use "Type your answer..." placeholder when question has no options
Extract hasAnsweredQuestionToolCall to a shared utility and use it in withoutActiveTurnAssistantHistory / getVisibleMessages so that assistant messages containing an answered ask_user_question are kept visible in the message list while a session is still running. Previously all assistant messages from the active turn were hidden, causing questions and their answers to disappear from chat history until the turn completed.
…lain-string fallback Replace the inline Q&A markdown block with a right-aligned answer bubble (styled like a user message) shown after the question is answered, and a "Question skipped" badge when the result carries no answers. Add a fallback for backends that return a plain string result (result.text / result.answer) rather than a structured answers map. Remove the isPending spinner from the ToolCallRow since the overlay in the footer already indicates active state.
…oll on question changes
- Fire onRespond without awaiting so the overlay never shows a separate
"Waiting for response..." state. The overlay stays frozen (same size and
content) until pendingQuestion updates from the server, at which point
React remounts the component via key={pendingQuestion.questionId}.
- Highlight the chosen option and replace its number badge with a spinner;
show a spinner on the pencil icon when the answer came from the text input.
- Skip now sends "skip" as the answer instead of aborting the agent, so the
LLM can continue. The X button still stops the session.
- Fix isQuestionSubmitting hardcoded to false — now passes questionResponsePending.
- Add footerScrollTrigger so the chat scrolls to the bottom whenever the
question overlay appears, updates, or disappears.
…inned header/footer
- Re-add inputValue to SlashCommandPreviewPopover anchor effect deps so the virtual anchor re-measures when typing shifts the chip's position - Clamp slash menu selectedIndex in onUpdate when filtered results shrink, matching the existing mention-menu clamping behavior - Return true (consume event) when Enter/Tab closes empty mention popup so the event does not propagate to insert a paragraph break - Mirror focus-on-dismiss effect in v2 workspace ChatInputFooter so the editor regains focus after the question overlay unmounts - Fix trailing-slash paths rendering empty label in ClickableFilePath by using || instead of ?? for the basename fallback
michalkopanski
force-pushed
the
chat-ux-enhancements
branch
from
7d87660 to
41e644d
Compare
Removes the fork-dependent preload wiring (metadata.skills → preloadSkills pass-through) that was a silent no-op on upstream mastracode. Keeps the SkillToolCall renderer so load_skill tool calls emitted by upstream's native skills system render with their own UI. Rewrites docs/skill-preload-feature.md to describe the upstream agent-autonomous model (SKILL.md discovery in .claude/skills, .agents/skills, .mastracode/skills).
Brings in upstream mastra's native skills system (search_skills + load_skill tools, SKILL.md discovery via skillPaths) which the SkillToolCall renderer in this PR now consumes for free. - mastracode: 0.14.0 → 0.15.0-alpha.3 - @mastra/core: 1.25.0 → 1.26.0-alpha.3 - @mastra/mcp: 1.3.1 → 1.5.1-alpha.1 Applied in apps/desktop, packages/chat, packages/host-service.
Auto-applied by biome/sherif.
…ts (superset-sh#3562) Memory leak and CPU spiral root-caused to `staleTime: 0, gcTime: 0` + 60fps polling: React Query can't dedupe or GC anything, and the render path churns allocations every 16ms. Restoring the React Query defaults (5min gcTime) fixes the leak. Server poll rate is independent of perceived stream smoothness — StreamingMessageText already reveals text client-side at 60fps from whatever buffer the server delivers. 4fps polling keeps that buffer fed with plenty of headroom. Also removes the `isRunning` invalidation effect — redundant when the query is polling. Builds on and supersedes superset-sh#3170 by @thepathmakerz, which diagnosed the same root cause. This version takes the subtractive path (-21 lines) instead of adaptive polling (+36). Closes superset-sh#3049
- Argument editing inline in chip: auto-focus on insert, right-arrow to exit, double-click to re-enter - Commands without argumentHint hide the colon/input entirely - Model command shows a dropdown of available models (no free-form text) - Chip input auto-sizes as user types (shrinks to content width) - Dropdown positioned above chip (side="top"), ArrowUp/Down navigate options, Tab/Enter commit selection - Menu reopens automatically when deleting value back to empty - Preview popover and select dropdown are mutually exclusive (preview only on hover/node-select, never while arg input is focused) - Focus shortcut hint moved inside TiptapPromptEditor (accepts focusShortcutText prop)
Contributor
There was a problem hiding this comment.
2 issues found across 7 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/components/Chat/ChatInterface/components/TiptapPromptEditor/SlashCommandNode.tsx">
<violation number="1" location="apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/SlashCommandNode.tsx:342">
P2: Parsed `argumentOptions` is not validated as a string array before later string operations, so malformed HTML can crash slash-command rendering.</violation>
</file>
<file name="apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/TiptapPromptEditor.tsx">
<violation number="1" location="apps/desktop/src/renderer/components/Chat/ChatInterface/components/TiptapPromptEditor/TiptapPromptEditor.tsx:643">
P2: Chip argument focus is tracked but not included in preview visibility, so the slash-command preview can close while editing a chip's arguments.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
|
You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment |
These tests replace every child component with a mock placeholder and assert on literal strings appearing in the rendered HTML. That tested mock plumbing, not behavior — every SUT import change broke them regardless of whether the actual render output changed, and the 'SUBAGENT_EXECUTION_MESSAGE' assertion was checking for a component this PR intentionally inlined. The useful bit (filter/ordering logic in messageListHelpers) is better covered by a direct unit test — leaving as a follow-up.
Kitenite
added a commit
that referenced
this pull request
Apr 18, 2026
Kitenite
added a commit
that referenced
this pull request
Apr 18, 2026
- notification-manager.test: update expected strings to match source (strings changed in #3039; test wasn't updated, CI was red on main too) - DashboardSidebarHeader: aria-label="Add repository" on icon-only dropdown triggers so screen readers announce them (tooltips don't count as accessible names) - docs/design/v2-project-create-import: correct v2Projects.create input shape (jwt-scoped { organizationId, name, slug, repoCloneUrl })
Kitenite
added a commit
that referenced
this pull request
Apr 19, 2026
* docs: consolidated weekly changelog — 2026-04-20 Supersedes #3206 (2026-04-06) and #3404 (2026-04-13), folding in this week's v2 workspace work so three weeks of shipped changes land in one post instead of three stale PRs. * docs(changelog): include chat UX overhaul (#3039) in consolidated post * docs(changelog): add compressed chat-ux hero screenshot * docs(changelog): refresh chat-ux hero and add v2 file tree screenshot * docs(changelog): add brand refresh screenshot * docs(changelog): add v2 diff viewer screenshot * docs(changelog): v2 workspace framed as early access, add screenshot * docs(changelog): rename title to 'v2 early access' * docs(changelog): frame v2 as a cloud-aimed rebuild — terminal rewrite, IDE architecture * docs(changelog): drop articles in title * docs(changelog): light trim — drop redundant lead-ins and filler words
Kitenite
added a commit
that referenced
this pull request
Apr 20, 2026
* docs: v2 project create/import design + plan
Simplified redesign after PR review. Collapses the earlier three-signal
backing model (cloud + per-host cloud signal + local) into two signals
(cloud + local-only), removes the v2_host_projects cloud table and
Electric sync, drops per-row state decoration on the sidebar, and moves
backing checks to action time (workspace-create modal, error paths).
* feat(trpc): v2Projects.findByGitHubRemote + jwt-scoped create
Adds the cloud-side matcher used by host-service's folder-first import
flow: given a clone URL, returns candidate projects the user has access
to whose GitHub repo matches (case-insensitively). Named findByGitHubRemote
(not findByRemote) because the match is GitHub-specific.
v2Projects.create switches to jwtProcedure with an explicit
organizationId + repoCloneUrl, matching the shape host-service needs to
call from project.create. No existing callers.
parseGitHubRemote moves from packages/host-service to packages/shared so
both cloud tRPC and host-service consume the same implementation.
* feat(host-service): project.create / setup / list / findByPath / remove
Full create/import lifecycle in host-service:
- project.list — DB read of host-service.projects. Pure, no filesystem
probing. Stale paths surface via operation errors, not proactive checks.
- project.findByPath — validate git root, read remote, forward to cloud
v2Projects.findByGitHubRemote. Backs the folder-first import picker.
- project.create — discriminated-union mode (empty/clone/importLocal/
template); Phase 1 ships clone + importLocal only, empty and template
throw NOT_IMPLEMENTED.
- project.setup — discriminated-union mode (clone/import) with
acknowledgeWorkspaceInvalidation gate on the re-point case.
- project.remove — local worktree + repo dir teardown.
Cloud backing (v2_host_projects) is intentionally absent: there is no
per-host cloud signal in this design. Backing is a local-only concept,
checked at action time.
Adds ProjectNotSetupCause to the error formatter so the renderer can
catch throws from workspace.create (next commit) and open the Pin & Set
Up modal inline.
* feat(desktop): add-repository modals at dashboard layout level
Three flows for getting projects onto this device:
- New project — clone a GitHub URL into a chosen parent directory.
Drives project.create(mode=clone).
- Import existing folder — native picker → project.findByPath branches on
candidate count. 0 → name + create (importLocal). 1, not set up here →
auto-advance to project.setup. 1, already set up → destructive re-point
confirmation. >1 → picker modal.
- Pin & set up — clone an existing cloud project onto this device.
Drives project.setup(mode=clone), with forceRepoint entry for repair.
All three modals are mounted once at the dashboard layout level via
AddRepositoryModals, and opened through a small zustand store. Sidebar
header "Add repository" dropdown triggers New project / Import folder.
* feat(desktop): workspaces-tab Available section + folder-first import trigger
Lists cloud projects in the user's active org that aren't pinned
locally. Pin & set up per row runs project.setup. Header dropdown
("Add repository") mirrors the sidebar — "+ New project" +
"Import existing folder." Entry points route through the dashboard-level
AddRepositoryModals via the shared zustand store.
useAvailableV2Projects powers the section: antijoin
v2Projects ∖ v2SidebarProjects scoped to the active organization,
with the existing v2-workspaces search filter applied.
* feat(desktop): workspace-create inline setup + remote-device stub
- Host-service workspaceCreation.{create,checkout,adopt} throw
PROJECT_NOT_SETUP (PRECONDITION_FAILED + cause { kind, projectId })
when this host has no local project row. No more silent auto-clone
into ~/.superset/repos/ — the user explicitly picks where to clone.
- Pending workspace-create page intercepts data.projectNotSetup on the
error, opens the Pin & set up modal pre-filled with the project, and
registers a one-shot onSuccess callback to retry the original intent
once setup resolves. The pending row stays in "creating" through the
modal so the UI doesn't flicker to failed.
- Clicking a remote-device workspace row lands on the new
WorkspaceNotOnThisHostState stub: explains the workspace lives on
another host, offers "Set up here" (opens Pin & set up for the
project) or "Browse workspaces." V2 workspace page checks
host.machineId via live query and renders the stub before mounting
the pane tree, which would otherwise crash on a foreign worktree.
* Fix infinite import
* fix: pre-existing notification test, a11y labels, design doc shape
- notification-manager.test: update expected strings to match source
(strings changed in #3039; test wasn't updated, CI was red on main too)
- DashboardSidebarHeader: aria-label="Add repository" on icon-only
dropdown triggers so screen readers announce them (tooltips don't
count as accessible names)
- docs/design/v2-project-create-import: correct v2Projects.create input
shape (jwt-scoped { organizationId, name, slug, repoCloneUrl })
* feat(desktop): unify new-workspace pickers + link popovers, strip chat link UI
- Unify DevicePicker / ProjectPickerPill / CompareBaseBranchPicker to a
shared FORM_PICKER_TRIGGER_CLASS: no background, h-[22px], text-[11px]
text-muted-foreground, size-3 icons, align="start" dropdowns. Bump the
project trigger thumbnail to size-4; drop the leftover `!` override
(twMerge handles it).
- DevicePicker: icon-only trigger (aria-label + title surface the name).
- Rewrite IssueLinkCommand / PRLinkCommand / GitHubIssueLinkCommand to
one codepath each: accept a button as `children`, wrap it in
PopoverTrigger, own their open state internally. No more shared
plusMenuRef, no more external open/onOpenChange/anchorRef coordination,
no manual onPointerDownOutside anchor-guard — Radix handles toggle and
dismiss natively so clicking a trigger while its popover is open
closes it like every other picker.
- v2 NewWorkspace PromptGroup: drop the three popover-open useStates +
plusMenuRef + manual toggle handlers. AttachmentButtons becomes a
layout shell that renders the three trigger elements as props; each
wraps a shared LinkTrigger (tooltip + pill button).
- Chat (v1 + v2) + v1 NewWorkspace PromptGroup: remove the link-issue
popover wiring (IssueLinkCommand usage, ChatShortcuts' onLinkIssue
callback). PlusMenu in chat collapses from a dropdown with
attach/link options to a plain attachment button.
- Temporarily disable v2 ChatPane render: it predates this PR and is
missing ChatServiceProvider (introduced in PR #3088), so
chatServiceTrpc has no context in TiptapPromptEditor. Replaced with a
"Chat pane is temporarily disabled" placeholder; original render body
commented out for quick restoration.
* feat(desktop): host-scoped project picker with Available / Needs setup sections
The v2 new-workspace picker was listing every cloud project the user
had access to, regardless of whether it was set up on the selected
device. That produced the PROJECT_NOT_SETUP error path on submit —
reviewers flagged the pending-row-stuck-in-creating fallout as a P1.
Root-cause fix: split the project list by selected-host availability.
- `useHostProjectIds(hostTarget)` queries host-service `project.list`
on the chosen device (local via activeHostUrl, remote via relay) and
returns the set of set-up project IDs.
- PromptGroup splits `recentProjects` into `availableProjects` +
`needSetupProjects` using that set; changing the device refetches.
- ProjectPickerPill renders two CommandGroup sections: Available (click
selects) and Needs setup (click opens Pin & set up for that project).
- Pin & set up already invalidates `["project", "list", activeHostUrl]`
on success, so after setup the project flips to Available — user
picks it and continues normally.
While `project.list` is loading or errors, everything falls back to
Available — picker stays usable; any real failure surfaces via the
existing workspace-create error path.
* lint
* fix(desktop): IssueLinkCommand uncontrolled close + PlusMenu aria-label
- IssueLinkCommand: the refactored popover-trigger API made `open` and
`onOpenChange` optional so callers (v2 PromptGroup) could let Radix
manage state. But `handleSelect` only fired the optional controlled
callback, so in uncontrolled mode the popover never closed after
picking an issue. Track state ourselves via a controllable-state
pattern: internal `useState` when the prop is absent, caller's value
when passed. `setOpen` always writes through, so close-on-select
works in both modes.
- PlusMenu: add aria-label="Add attachment" to the icon-only trigger.
Radix Tooltip sets aria-describedby on the trigger, not
aria-labelledby, so screen readers previously announced it as an
unlabeled button.
* refactor(desktop): drop controlled-open props from IssueLinkCommand; extend aria-label fix
- IssueLinkCommand: only caller passes `onSelect + children`, so the
optional open/onOpenChange pass-through was dead code. Simplify to
always-internal state. Radix Popover has no imperative close from
inside its content — owning state is the canonical shadcn/cmdk
pattern, not scaffolding.
- AttachmentButtons (v2): add aria-label to the shared LinkTrigger (so
Link issue / Link GitHub issue / Link pull request all announce a
name) and to the paperclip. Same fix as PlusMenu — Radix Tooltip
sets aria-describedby on the trigger, not aria-labelledby, so
tooltip-only buttons read as unlabeled to screen readers.
* fix(trpc): scope v2Project.findByGitHubRemote + modal picker to active org
host-service is pinned to a single organization at boot (env.ORGANIZATION_ID);
its local projects table has no orgId column. Project discovery was leaking
across orgs:
- v2Project.findByGitHubRemote used ctx.organizationIds (plural, all
accessible orgs). The folder-first picker would surface candidates from
orgs the current host can't set up, producing a confusing NOT_FOUND when
host-service then called v2Project.get with its own org.
- DashboardNewWorkspaceModalContent queried collections.v2Projects with no
org filter. Same over-fetch, same downstream failure.
Align both with the rest of the codebase (v2Project.get / create,
useAvailableV2Projects, useWorkspaceHostOptions) which take/filter by an
explicit active orgId:
- findByGitHubRemote: add organizationId input, authorize it against
ctx.organizationIds (same shape as get/create), filter candidates by it.
- host-service project.findByPath: pass ctx.organizationId through.
- DashboardNewWorkspaceModalContent: .where(eq(projects.organizationId,
activeOrganizationId)) on the live query, matching useAvailableV2Projects.
* Lint
* fix(host-service): clone-then-cloud in project.createFromClone, rollback on cloud failure
Matches the local-first-then-cloud pattern already used by
workspace.create (workspace-creation.ts:860-918, which git-worktree-adds
first then registers cloud with a rollback on failure).
Previously createFromClone called v2Project.create before cloneRepoInto,
so any clone failure (network, bad URL, auth, dir collision) left a cloud
v2_projects row with nothing local backing it on any host. Retrying the
flow with corrected input accumulated more orphans.
Reorder: clone first, register cloud in try/catch, rmSync the freshly-
created clone if cloud-create or persistLocalProject throws.
* fix(host-service): move project.create visibility into GitHub-provisioning modes
Only empty + template modes provision a new GitHub repo and need to tell
the GitHub App whether it should be private or public. clone +
importLocal reuse an existing remote where visibility is already set —
the top-level field was required but ignored for those two paths.
Move `visibility: z.enum(["private", "public"])` into the empty and
template variants of the discriminated union. Drop it from clone/
importLocal callers. Update design doc to match.
* refactor(desktop): use Radix composition for link-command tooltips, drop dead chat-link wiring
Responds to saddlepaddle + Kitenite reviews on PR #3566.
- IssueLinkCommand / PRLinkCommand / GitHubIssueLinkCommand now own the
Popover + Tooltip composition internally via
`PopoverTrigger asChild > TooltipTrigger asChild`. Callers pass a plain
PromptInputButton + a tooltipLabel prop. Removes the LinkTrigger
forwardRef + `{...rest}` spread trick that was sneaking Popover props
through an intermediate Tooltip wrapper.
- Delete the misleading JSDoc at IssueLinkCommand claiming Radix can't be
closed imperatively — PopoverClose exists; the controlled-open pattern
we use is just shadcn's canonical combobox.
- Drop orphaned `_issueLinkOpen` / `_addLinkedIssue` + the
`setIssueLinkOpen` prop threaded through ChatShortcuts in both v1 and
v2 chat, plus the same dead state in v1 NewWorkspaceModal PromptGroup.
- Retire the CHAT_LINK_ISSUE hotkey entry — its only consumer was the
dead setIssueLinkOpen toggle.
* chore: trim past-state narration + what-describing comments
- Drop "previously this did X" / "introduced in PR #3088" / commented-out
renderPane block in v2 usePaneRegistry chat pane.
- Collapse JSDocs that only restated the function's name (ParentDirectoryPicker,
AddRepositoryModals layout blurb, per-method docs on UseFolderFirstImportResult,
persistLocalProject).
- Tighten the explanatory comments that still earn their keep (pending
PROJECT_NOT_SETUP interceptor, PinAndSetupModal conflict state, store
onSuccess / forceRepoint prop docs).
* refactor(desktop): strip v2 discovery/recovery surface to MVP
Two rules for v1:
- Sidebar = pinned projects.
- Workspaces tab = every workspace in the user's active org.
Code deletes:
- V2AvailableProjectsSection, useAvailableV2Projects, useHostProjectIds.
- v2UsersHosts innerJoin in useAccessibleV2Workspaces — tab no longer
drops rows when host access changes.
- Available-section wiring in V2WorkspacesList + v2-workspaces/page.tsx.
- Available / Needs-setup split in ProjectPickerPill and the
openPinAndSetup bridge in PromptGroup. Also removes the wrong-host
bug (cubic AF_o, saddlepaddle CXVQ) as dead code.
- PROJECT_NOT_SETUP recovery loop in the pending page — failure is a
plain toast now.
Docs realigned:
- design/v2-project-create-import.md opens with the two rules and
moves Available / inline setup / backing signals to an explicit
"Out of scope for v1" block.
- plans/20260417-v2-project-create-import-impl.md mirrors the same
deferrals; Phase-1 checklist is now all checked.
Net −537 lines. Typecheck + lint clean.
* refactor(desktop): open remote-host workspaces without gating
Previously any workspace whose hostMachineId didn't match the local
machine landed on a WorkspaceNotOnThisHostState stub. That hid the
workspace from the user entirely when the whole point is to let them
see it. Delete the gate, delete the stub component, and let the
workspace page render for any host. Operations that assume local
filesystem (terminal spawn, local git) fail at the point they run.
Also slims the page's live query — projectGithubOwner, projectName,
hostMachineId etc. were only fed into the stub.
Design doc + plan updated to reflect the no-gating posture.
Resolves saddlepaddle CTvC.
* refactor(desktop): extract FormPickerTrigger component
Addresses saddlepaddle CWlq — the shared style for the three top-of-modal
pickers (Device / Project / Branch) lived as a string constant in
types.ts, which is an odd place for a className and doesn't compose.
Promote it to a named FormPickerTrigger component that encapsulates the
base button styles and accepts extra className + native button props.
The three call sites lose their raw <button type="button"> +
backtick-composed classNames.
Drops FORM_PICKER_TRIGGER_CLASS from types.ts.
* refactor(desktop): remove dead PinAndSetupModal + async-hygiene sweep
PinAndSetupModal had zero remaining callers after the MVP cut — the
pending-page PROJECT_NOT_SETUP interceptor and the Available-section
"Pin & set up" button were the only two. Delete the whole modal,
its store action, useOpenPinAndSetupModal hook, PinAndSetupTarget
type, and the forceRepoint plumbing that existed only to support it.
Also addresses the async-hygiene nits on the surviving surfaces:
- useFolderFirstImport.start wraps selectDirectory.mutateAsync in
try/catch → reportError (coderabbit nmS, cubic op5).
- ParentDirectoryPicker.handleBrowse wraps the same (cubic op8).
- AddRepositoryModals effect adds .catch on startRef.current()
(cubic oqE).
- FolderFirstImportModal keys CandidatePickerContent on repoPath so
selectedId resets per import (coderabbit nmM).
Docs + plan updated to reflect the removed modal + ENOENT recovery
deferral.
Net −205 lines. Typecheck + lint clean.
* refactor(host-service): reject re-pointing instead of confirming it
v1 has no re-point UX. project.setup now treats an existing row as:
- same resolved path → no-op success (idempotent; fixes the false
CONFLICT that cubic/coderabbit flagged on same-path setup).
- different path → CONFLICT with the existing path in the message,
no escape hatch. User must project.remove first if they genuinely
want to move the project.
Drops `acknowledgeWorkspaceInvalidation` from the input, the ack
branch of the CONFLICT guard, and the setupFromClone/setupFromImport
helpers + SetupContext type in handlers.ts (the setup path is small
enough to inline).
Client drops the confirm-repoint state, confirmRepoint method,
ConfirmRepointContent component, and the conflict branch in
SetupInvokeResult — none of which have anything to retry against.
Also fixes the TOCTOU race in cloneRepoInto: replaces
existsSync + rmSync-on-error with mkdirSync (atomic claim) +
rmSync-on-error, so clone failure can't delete a directory this
process didn't create.
Resolves coderabbit nmb, nmd, and cubic oqN.
* fix(desktop): unify workspaces-tab empty state
The onboarding "No workspaces yet" check was reading already-filtered
pinned/others counts, so a search that matched nothing landed on the
onboarding copy instead of the clear-filters UI.
Collapse to a single !hasAnyMatches branch that picks copy + icon
based on hasActiveFilters. Drops the bogus hasAnyWorkspaces check.
Resolves cubic CrwE.
* revert queries
* Clean up dead code
* refactor(desktop): port v1 new-project UI into NewProjectModal
Replace the bespoke name + clone-URL + parent-picker form with v1's
new-project page layout: a Location row (text input + browse button),
three mode tiles (Clone/Empty/Template), and a per-mode form. Only
Clone is wired up; Empty + Template carry "(coming soon)" since v2
project.create throws NOT_IMPLEMENTED for them.
Location auto-populates to ~/.superset/projects via window.getHomeDir.
Project name is derived from the clone URL's last segment so the form
matches v1 (no explicit name field).
ParentDirectoryPicker deleted — the inline Input + folder button
replaces it and there's no other caller.
* feat(db/trpc): decouple v2 projects from GitHub App installs
v2Projects previously required a non-null githubRepositoryId, which
gated project creation on the org having installed the repo via the
GitHub App. Cloning any other repo (public, not installed, or non-
matching) failed at the cloud step after a successful local clone.
Changes:
- githubRepositoryId becomes nullable with ON DELETE SET NULL,
matching v1's projects table.
- repoCloneUrl is added as the canonical source of truth for the
remote URL. Also nullable so empty-mode / local-only projects
without a remote can coexist.
- UNIQUE(organization_id, lower(repo_clone_url)) prevents two
projects from claiming the same repo in one org. NULLs don't
collide, so URL-less projects still work.
- v2Project.create accepts an optional repoCloneUrl, canonicalizes
via parseGitHubRemote, and links a matching github_repositories
row case-insensitively when one exists. Unique-violation (23505)
surfaces as CONFLICT with per-constraint messaging.
- v2Project.findByGitHubRemote matches on v2Projects.repoCloneUrl
directly instead of joining through the installation table, so
unlinked projects are discoverable.
- v2Project.get drops the derived repoCloneUrl — consumers read the
stored column or the joined githubRepository directly.
Migration 0034 bundles all five schema changes. Nullable-safe: no
backfill required for existing rows.
* No candidate thing
* feat(desktop): flag projects not set up on selected host in new-workspace modal
After picking a host in DevicePicker, each project in ProjectPickerPill
shows an amber warning triangle when that host doesn't have the project
set up locally. A matching "Project needs to be set up" note appears
next to the ⌘↵ hint when the currently-selected project needs setup,
so the user sees the blocker before submitting.
Setup state comes from a per-host project.list query (re-added to the
host-service router). The RPC is resolved through the standard
getHostServiceClientByUrl path — local uses activeHostUrl, remote/cloud
goes through the relay. If the host is unreachable we treat setup as
unknown and hide the indicator rather than falsely flagging everything.
Submit path is unchanged: picking a not-set-up project still fires
workspace.create, which throws PROJECT_NOT_SETUP and surfaces as the
existing toast. Inline setup UX is still deferred.
* chore: biome format fix + sync design doc to workspaces-tab filter
- git.ts: biome wants the ghMsg ternary wrapped; main's 27e243b added
the catch block and the CI biome check caught it post-merge.
- design doc: the workspaces tab code filters to hosts the user is
linked to via v2_users_hosts, not every workspace in the org. Update
wording to match what shipped; note teammate workspaces on unshared
hosts are not surfaced in v1.
* docs: move v2 project create/import plan to plans/done
Plan is shipped — move per AGENTS.md rule 7 and drop the rewrite/history
notes in both plan and design doc since the PR body is the canonical
record of what was cut.
* docs: drop rewrite/history notes from plan and design doc
Captured in PR body instead.
* chore: trim restating/navigational comments in project handlers
This was referenced Apr 23, 2026
MocA-Love
added a commit
to MocA-Love/superset
that referenced
this pull request
Apr 24, 2026
…uperset-sh#3039 test が期待する upstream 新仕様 ("Awaiting Response" / "is waiting for your reply") に実装を合わせる。cherry-pick 取り込み時に test のみ upstream 側に置き換わり、実装が fork 旧仕様 ("Input Needed" / "needs your attention") のままだった不整合を修正。
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
2 participants
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
A comprehensive overhaul of the chat UI/UX — revamped prompt input, new interaction patterns, a unified code display system, and a set of shared UI primitives that establish patterns for all future tool call renderers.
Related PRs
UX Changes
Prompt Input (Tiptap)
Replaced the plain `<textarea>` with a ProseMirror-based rich text editor:
Slash Command Chip Argument Editing
Tool Call Redesign
ask_user / Question Flow
File Read Display
Workspace Status & Notifications
Scroll Behaviour