feat(desktop): add slash commands and chat UI improvements#1309
Conversation
|
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:
📝 WalkthroughWalkthroughAdds slash-command support: a TRPC procedure to discover Markdown-defined commands, client hooks and UI components to present and navigate commands, ChatInterface integration to invoke commands, and related stream/cleanup adjustments for assistant messages and deferred client disposal. Changes
Sequence Diagram(s)sequenceDiagram
participant Renderer as Renderer UI
participant Hook as useSlashCommands
participant TRPC as Desktop TRPC (ai-chat)
participant FS as Host FS (node)
Renderer->>Hook: input starts with "/"
Hook->>TRPC: query getSlashCommands({ cwd })
TRPC->>FS: scan `.claude/commands` in cwd and home
FS-->>TRPC: command entries (deduped)
TRPC-->>Hook: commands[]
Hook-->>Renderer: filteredCommands, isOpen, selectedIndex
Renderer->>Renderer: user navigates/selects command
Renderer->>Hook: resolveCommandAction(selected)
alt command has args
Hook-->>Renderer: insert "/command " (do not send)
else no args
Hook-->>Renderer: send "/command" as message
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@apps/desktop/src/lib/trpc/routers/ai-chat/index.ts`:
- Around line 27-42: The getSlashCommands procedure currently casts external
JSON and swallows errors; change it to accept the trpc context (e.g., query(({
ctx }) => ...)) so you can log, add a fetch timeout using AbortController to
avoid hangs, validate the response with a Zod schema (schema for { commands:
Array<{ name:string; description?:string; argumentHint?:string }> }) and parse
the JSON with safeParse, and on any validation/fetch/non-ok error call the
logger (e.g., ctx.logger or processLogger) with context and return the explicit
fallback { commands: [] } instead of silently swallowing errors.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/hooks/useSlashCommands/useSlashCommands.ts`:
- Around line 66-72: Replace the after-render useEffect block that compares
prevQuery.current to query with a during-render conditional: inside the render
body check if prevQuery.current !== query, then set prevQuery.current = query
and call setSelectedIndex(0) immediately so the state is adjusted before commit;
additionally, ensure selectedIndex is clamped to the new filteredCommands bounds
by checking filteredCommands.length and calling setSelectedIndex(Math.max(0,
Math.min(selectedIndex, filteredCommands.length - 1))) when needed; reference
the existing symbols prevQuery, query, setSelectedIndex, selectedIndex, and
filteredCommands to locate and update the logic.
In `@apps/streams/src/claude-agent.ts`:
- Around line 264-276: Replace the silent catch and add a promise sentinel to
deduplicate in-flight calls: introduce a module-level cachedCommandsPromise
variable and, when cachedCommands is null and cachedCommandsPromise is falsy,
set cachedCommandsPromise = result.supportedCommands().then(cmds => {
cachedCommands = cmds.map(...); cachedCommandsPromise = undefined; }).catch(err
=> { processLogger.error("Failed to populate supportedCommands cache", err);
cachedCommandsPromise = undefined; }); ensure other request handlers check
cachedCommandsPromise (and skip starting a new fetch while it exists) so
simultaneous requests don't start multiple supportedCommands() calls; do not
swallow errors—log them with context via processLogger.error.
🧹 Nitpick comments (5)
packages/durable-session/src/react/use-durable-chat.ts (1)
187-192: InconsistentisDisposedguard: check should be inside the microtaskAt lines 244–247 (cleanup effect), you correctly recheck
!c.isDisposedinside the microtask. But here (and at lines 200–203) the guard is only outsidequeueMicrotask. Between scheduling and execution, another path (e.g., Strict Mode re-render or effect cleanup) could dispose the client first, leading to a double-dispose.For consistency and safety, move the guard inside:
Proposed fix
const prev = clientRef.current?.client; - if (prev && !prev.isDisposed) { - // Defer disposal via microtask — dispose() triggers collection events - // which call setState, so it must not run during render or useEffect. - queueMicrotask(() => prev.dispose()); + if (prev) { + queueMicrotask(() => { + if (!prev.isDisposed) prev.dispose(); + }); }Apply the same pattern at lines 200–203.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/SlashCommandMenu/SlashCommandMenu.tsx (1)
26-61: Consider adding ARIA attributes for accessibility.The menu functions as a listbox with keyboard navigation handled by the parent, but it lacks semantic ARIA roles. Screen readers won't announce the selected item or the menu's purpose without
role="listbox"on the container androle="option"/aria-selectedon each button.Sketch
- <div className="absolute bottom-full left-0 z-50 mb-2 w-full max-h-[200px] overflow-y-auto rounded-lg border border-border bg-popover p-1 shadow-md"> + <div role="listbox" className="absolute bottom-full left-0 z-50 mb-2 w-full max-h-[200px] overflow-y-auto rounded-lg border border-border bg-popover p-1 shadow-md"> {commands.map((cmd, index) => ( <button key={cmd.name} ref={index === selectedIndex ? selectedRef : undefined} type="button" + role="option" + aria-selected={index === selectedIndex} className={`flex w-full cursor-pointer flex-col gap-0.5 rounded-md px-3 py-2 text-left transition-colors ${apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/ChatInterface.tsx (1)
266-268:/clearcommand silently does nothing.
handleClearis a no-op, so users who pick/clearfrom the command menu will see no feedback and no effect. Even before wiring the actual reset logic, consider at minimum logging a message or showing a toast so it's not a silent dead-end.Would you like me to open an issue to track wiring the
/clearcommand to an actual session/conversation reset?apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/hooks/useSlashCommands/useSlashCommands.ts (2)
42-44: Extract the stale-time constant.
5 * 60 * 1000is a magic number. As per coding guidelines, "Extract hardcoded magic numbers, strings, and enums to named constants at module top instead of leaving them inline in logic."Suggested fix
Add at the top of the file:
+const COMMANDS_STALE_TIME_MS = 5 * 60 * 1000;Then use it:
const { data } = electronTrpc.aiChat.getSlashCommands.useQuery(undefined, { - staleTime: 5 * 60 * 1000, + staleTime: COMMANDS_STALE_TIME_MS, });
4-39: Define a Zod schema for SlashCommand in the tRPC router and use it as the single source of truth for both the type and validation.The
SlashCommandinterface andDEFAULT_COMMANDSare currently duplicated acrossapps/streams/src/claude-agent.tsand this file, creating a maintenance burden. Additionally, thegetSlashCommandstRPC procedure lacks Zod validation despite fetching untrusted external data—it uses a type assertion instead. The solution is to:
- Define
SlashCommandSchemausing Zod in the tRPC router (apps/desktop/src/lib/trpc/routers/ai-chat/index.ts)- Use
z.infer<typeof SlashCommandSchema>in the renderer to eliminate the duplicate type definition- Use the schema to validate the response from
/commandsbefore returning itThis makes the tRPC response schema the canonical definition, enforces validation at the boundary, and removes duplication.
| getSlashCommands: publicProcedure.query(async () => { | ||
| try { | ||
| const res = await fetch(`${CLAUDE_AGENT_URL}/commands`); | ||
| if (!res.ok) return { commands: [] }; | ||
| const data = (await res.json()) as { | ||
| commands: Array<{ | ||
| name: string; | ||
| description: string; | ||
| argumentHint: string; | ||
| }>; | ||
| }; | ||
| return { commands: data.commands ?? [] }; | ||
| } catch { | ||
| return { commands: [] }; | ||
| } | ||
| }), |
There was a problem hiding this comment.
Validate the external response and log errors instead of swallowing them.
Three concerns:
- Untrusted data: The
ascast on line 31 doesn't validate at runtime. If the agent returns an unexpected shape, malformed data propagates to the UI. Use Zod to parse the response. - Silent catch: The
catchblock silently returns{ commands: [] }with no logging, violating the guideline against swallowing errors. - No fetch timeout: If the Claude agent is unresponsive, this query hangs indefinitely.
Proposed fix
+const slashCommandsResponseSchema = z.object({
+ commands: z.array(
+ z.object({
+ name: z.string(),
+ description: z.string().optional().default(""),
+ argumentHint: z.string().optional().default(""),
+ }),
+ ).optional().default([]),
+});
+
getSlashCommands: publicProcedure.query(async () => {
try {
- const res = await fetch(`${CLAUDE_AGENT_URL}/commands`);
- if (!res.ok) return { commands: [] };
- const data = (await res.json()) as {
- commands: Array<{
- name: string;
- description: string;
- argumentHint: string;
- }>;
- };
- return { commands: data.commands ?? [] };
- } catch {
+ const res = await fetch(`${CLAUDE_AGENT_URL}/commands`, {
+ signal: AbortSignal.timeout(5000),
+ });
+ if (!res.ok) {
+ console.warn(`[ai-chat/getSlashCommands] Non-OK response: ${res.status}`);
+ return { commands: [] };
+ }
+ const raw = await res.json();
+ const data = slashCommandsResponseSchema.parse(raw);
+ return { commands: data.commands };
+ } catch (err) {
+ console.warn("[ai-chat/getSlashCommands] Failed to fetch commands:", err);
return { commands: [] };
}
}),As per coding guidelines: "Validate external API data as untrusted by handling missing fields, unknown enums, and unexpected shapes with tolerant parsing and explicit fallbacks" and "Never swallow errors silently; at minimum log errors with context before rethrowing or handling them explicitly."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| getSlashCommands: publicProcedure.query(async () => { | |
| try { | |
| const res = await fetch(`${CLAUDE_AGENT_URL}/commands`); | |
| if (!res.ok) return { commands: [] }; | |
| const data = (await res.json()) as { | |
| commands: Array<{ | |
| name: string; | |
| description: string; | |
| argumentHint: string; | |
| }>; | |
| }; | |
| return { commands: data.commands ?? [] }; | |
| } catch { | |
| return { commands: [] }; | |
| } | |
| }), | |
| const slashCommandsResponseSchema = z.object({ | |
| commands: z.array( | |
| z.object({ | |
| name: z.string(), | |
| description: z.string().optional().default(""), | |
| argumentHint: z.string().optional().default(""), | |
| }), | |
| ).optional().default([]), | |
| }); | |
| getSlashCommands: publicProcedure.query(async () => { | |
| try { | |
| const res = await fetch(`${CLAUDE_AGENT_URL}/commands`, { | |
| signal: AbortSignal.timeout(5000), | |
| }); | |
| if (!res.ok) { | |
| console.warn(`[ai-chat/getSlashCommands] Non-OK response: ${res.status}`); | |
| return { commands: [] }; | |
| } | |
| const raw = await res.json(); | |
| const data = slashCommandsResponseSchema.parse(raw); | |
| return { commands: data.commands }; | |
| } catch (err) { | |
| console.warn("[ai-chat/getSlashCommands] Failed to fetch commands:", err); | |
| return { commands: [] }; | |
| } | |
| }), |
🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/ai-chat/index.ts` around lines 27 - 42, The
getSlashCommands procedure currently casts external JSON and swallows errors;
change it to accept the trpc context (e.g., query(({ ctx }) => ...)) so you can
log, add a fetch timeout using AbortController to avoid hangs, validate the
response with a Zod schema (schema for { commands: Array<{ name:string;
description?:string; argumentHint?:string }> }) and parse the JSON with
safeParse, and on any validation/fetch/non-ok error call the logger (e.g.,
ctx.logger or processLogger) with context and return the explicit fallback {
commands: [] } instead of silently swallowing errors.
| const prevQuery = useRef(query); | ||
| useEffect(() => { | ||
| if (prevQuery.current !== query) { | ||
| setSelectedIndex(0); | ||
| prevQuery.current = query; | ||
| } | ||
| }, [query]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace useEffect with during-render state adjustment to avoid a stale selectedIndex frame.
The useEffect fires after render, so there is one render cycle where selectedIndex can be stale (and potentially out of bounds for the new filteredCommands). React supports calling setState conditionally during render for exactly this "adjust state when props/derived values change" pattern — it re-renders before committing to the DOM.
Suggested fix
const prevQuery = useRef(query);
- useEffect(() => {
- if (prevQuery.current !== query) {
- setSelectedIndex(0);
- prevQuery.current = query;
- }
- }, [query]);
+ if (prevQuery.current !== query) {
+ setSelectedIndex(0);
+ prevQuery.current = query;
+ }This also aligns with the coding guideline: "Do not use effect unless absolutely necessary." As per coding guidelines, "Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const prevQuery = useRef(query); | |
| useEffect(() => { | |
| if (prevQuery.current !== query) { | |
| setSelectedIndex(0); | |
| prevQuery.current = query; | |
| } | |
| }, [query]); | |
| const prevQuery = useRef(query); | |
| if (prevQuery.current !== query) { | |
| setSelectedIndex(0); | |
| prevQuery.current = query; | |
| } |
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/hooks/useSlashCommands/useSlashCommands.ts`
around lines 66 - 72, Replace the after-render useEffect block that compares
prevQuery.current to query with a during-render conditional: inside the render
body check if prevQuery.current !== query, then set prevQuery.current = query
and call setSelectedIndex(0) immediately so the state is adjusted before commit;
additionally, ensure selectedIndex is clamped to the new filteredCommands bounds
by checking filteredCommands.length and calling setSelectedIndex(Math.max(0,
Math.min(selectedIndex, filteredCommands.length - 1))) when needed; reference
the existing symbols prevQuery, query, setSelectedIndex, selectedIndex, and
filteredCommands to locate and update the logic.
| // Populate slash-command cache on first query (fire-and-forget) | ||
| if (!cachedCommands) { | ||
| result | ||
| .supportedCommands() | ||
| .then((cmds) => { | ||
| cachedCommands = cmds.map((cmd) => ({ | ||
| name: cmd.name ?? "", | ||
| description: cmd.description ?? "", | ||
| argumentHint: cmd.argumentHint ?? "", | ||
| })); | ||
| }) | ||
| .catch(() => {}); | ||
| } |
There was a problem hiding this comment.
Silent .catch(() => {}) swallows errors; minor thundering-herd on cache init.
- Silent catch: If
supportedCommands()fails, the error is silently discarded andcachedCommandsstaysnull, meaning every subsequentPOST /will retry the call — but without any log to diagnose why commands never populate. - Multiple in-flight calls: Before the first
supportedCommands()resolves, concurrent requests each seecachedCommands === nulland fire additional calls. Consider using a promise sentinel to deduplicate.
Proposed fix
-let cachedCommands: SlashCommand[] | null = null;
+let cachedCommands: SlashCommand[] | null = null;
+let commandsFetchPromise: Promise<void> | null = null;
// Populate slash-command cache on first query (fire-and-forget)
-if (!cachedCommands) {
- result
+if (!cachedCommands && !commandsFetchPromise) {
+ commandsFetchPromise = result
.supportedCommands()
.then((cmds) => {
cachedCommands = cmds.map((cmd) => ({
name: cmd.name ?? "",
description: cmd.description ?? "",
argumentHint: cmd.argumentHint ?? "",
}));
})
- .catch(() => {});
+ .catch((err) => {
+ console.warn("[claude-agent] Failed to fetch supported commands:", err);
+ })
+ .finally(() => {
+ commandsFetchPromise = null;
+ });
}As per coding guidelines: "Never swallow errors silently; at minimum log errors with context before rethrowing or handling them explicitly."
🤖 Prompt for AI Agents
In `@apps/streams/src/claude-agent.ts` around lines 264 - 276, Replace the silent
catch and add a promise sentinel to deduplicate in-flight calls: introduce a
module-level cachedCommandsPromise variable and, when cachedCommands is null and
cachedCommandsPromise is falsy, set cachedCommandsPromise =
result.supportedCommands().then(cmds => { cachedCommands = cmds.map(...);
cachedCommandsPromise = undefined; }).catch(err => { processLogger.error("Failed
to populate supportedCommands cache", err); cachedCommandsPromise = undefined;
}); ensure other request handlers check cachedCommandsPromise (and skip starting
a new fetch while it exists) so simultaneous requests don't start multiple
supportedCommands() calls; do not swallow errors—log them with context via
processLogger.error.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.claude/commands/create-pr.md (1)
10-24:⚠️ Potential issue | 🟠 MajorInconsistency: Step 1 checks for
main/master, but Step 2 hardcodesorigin/main.Step 1 validates that the user is not on
main/masterbranch (line 10), acknowledging that repositories may use either as the default branch. However, Step 2 now hardcodesorigin/mainin all git commands (lines 17, 21-24). If a repository usesmasteras its default branch, the fetch and subsequent diff/log commands will fail or produce incorrect results.🔧 Suggested approach
Detect the default branch dynamically before running the diff commands:
## Step 2: Analyze Changes First, determine the base branch to diff against: +- Detect the default branch: `git symbolic-ref refs/remotes/origin/HEAD | sed 's@^refs/remotes/origin/@@'` +- Let `BASE` be the detected branch (typically `main` or `master`) -- Run `git fetch origin main --quiet` to ensure the remote ref is up to date -- Use `origin/main` (not local `main`) as the base for all diff/log commands — local `main` may be stale +- Run `git fetch origin $BASE --quiet` to ensure the remote ref is up to date +- Use `origin/$BASE` (not local `$BASE`) as the base for all diff/log commands — local branch may be stale Run in parallel: -- `git log origin/main..HEAD --oneline` — commit history -- `git log origin/main..HEAD --format="%B---"` — full commit messages for context -- `git diff origin/main...HEAD --stat` — file change overview -- `git diff origin/main...HEAD` — full diff +- `git log origin/$BASE..HEAD --oneline` — commit history +- `git log origin/$BASE..HEAD --format="%B---"` — full commit messages for context +- `git diff origin/$BASE...HEAD --stat` — file change overview +- `git diff origin/$BASE...HEAD` — full diff
🧹 Nitpick comments (3)
.claude/commands/create-pr.md (1)
17-17: Minor: hyphenate "up-to-date" when used as compound adjective.When "up to date" modifies a noun ("remote ref"), it should be hyphenated: "up-to-date."
📝 Suggested fix
-- Run `git fetch origin main --quiet` to ensure the remote ref is up to date +- Run `git fetch origin main --quiet` to ensure the remote ref is up-to-dateapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/SlashCommandInput/SlashCommandInput.tsx (1)
44-76:slashCommandsobject in dependency array likely changes every render.The
useSlashCommandshook returns a plain object that's recreated each render, sohandleKeyDownCaptureis effectively never memoized. Consider destructuring the specific values you depend on:♻️ Suggested refactor
+ const { isOpen, filteredCommands, selectedIndex, navigateUp, navigateDown } = slashCommands; + const handleKeyDownCapture = useCallback( (e: React.KeyboardEvent) => { - if (!slashCommands.isOpen) return; + if (!isOpen) return; switch (e.key) { case "Escape": e.preventDefault(); e.stopPropagation(); textInput.setInput(""); break; case "Enter": case "Tab": { e.preventDefault(); e.stopPropagation(); - const cmd = - slashCommands.filteredCommands[slashCommands.selectedIndex]; + const cmd = filteredCommands[selectedIndex]; if (cmd) executeCommand(cmd); break; } case "ArrowUp": e.preventDefault(); e.stopPropagation(); - slashCommands.navigateUp(); + navigateUp(); break; case "ArrowDown": e.preventDefault(); e.stopPropagation(); - slashCommands.navigateDown(); + navigateDown(); break; } }, - [slashCommands, textInput, executeCommand], + [isOpen, filteredCommands, selectedIndex, navigateUp, navigateDown, textInput, executeCommand], );This also requires updating the JSX to use the destructured variables. Alternatively, the hook itself could be updated to return a stable (memoized) object.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/ChatInterface.tsx (1)
253-261:handleClearis a no-op — ensure it's tracked.The TODO on Line 260 notes this needs wiring to session/conversation reset. As-is, selecting the
/clearslash command will clear the input text (viaresolveCommandAction) but won't actually reset the conversation state.Would you like me to open an issue to track implementing the clear/reset functionality?
Adds filesystem scanning for custom commands instead of waiting for SDK cache. Also invalidates the command list when "/" is typed to ensure fresh results.
…n/main Align slash command menu with FileMention pattern using Radix Popover instead of manual absolute positioning. SlashCommandInput now wraps children with Popover+PopoverAnchor, composing cleanly with FileMention.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/desktop/src/lib/trpc/routers/ai-chat/index.ts`:
- Line 51: The empty catch after the block that calls readdirSync and
readFileSync swallows filesystem errors; replace the silent catch with explicit
error handling: catch the error object, log a descriptive message including the
operation and path (e.g., mentioning readdirSync/readFileSync and the
directory/file variables used), and then either rethrow or return a clear
fallback so callers can handle it; ensure you use the module's logger (or
console.error if no logger exists) and include the error stack/details for
diagnostics.
🧹 Nitpick comments (4)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/hooks/useSlashCommands/useSlashCommands.ts (2)
113-125:"clear"is a magic string — consider extracting to a named constant.
resolveCommandActionand potentially other call sites compare against the literal"clear". A shared constant (e.g.,CLEAR_COMMAND_NAME) would prevent silent breakage if the name changes.
50-58: Fallback silently hides fetch errors — surface or log them.When the query fails,
dataisundefinedand the hook silently falls back toDEFAULT_COMMANDSwithout any indication of an error. Consider destructuringerrorfrom the query and logging it so failures aren't invisible.Suggested fix
- const { data } = electronTrpc.aiChat.getSlashCommands.useQuery( + const { data, error: fetchError } = electronTrpc.aiChat.getSlashCommands.useQuery( { cwd }, { staleTime: 5 * 60 * 1000 }, ); + + if (fetchError) { + console.warn("[slash-commands] Failed to fetch commands:", fetchError.message); + } const commands = useMemo(() => {As per coding guidelines, "Never swallow errors silently; at minimum log errors with context before rethrowing or handling them explicitly."
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/SlashCommandInput/SlashCommandInput.tsx (1)
44-76:slashCommandsobject reference changes every render, causinghandleKeyDownCaptureto be recreated each time.
useSlashCommandsreturns a new object literal on every render, so listingslashCommandsas a dependency meansuseCallbacknever memoizes. Consider destructuring the specific values you need (isOpen,filteredCommands,selectedIndex,navigateUp,navigateDown) and listing those individually, or accept the current behavior since it's just an event handler prop on a<div>.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/ChatInterface.tsx (1)
255-256:handleClearis a no-op —/clearcommand will clear the input text but won't reset the conversation.Users selecting
/clearwill see their input vanish (viatextInput.setInput("")inSlashCommandInput) but the conversation and session state remain unchanged. This could be confusing.Would you like me to open an issue to track implementing the actual session/conversation reset for the
/clearcommand?
| argumentHint: "", | ||
| }); | ||
| } | ||
| } catch {} |
There was a problem hiding this comment.
Silent catch swallows filesystem errors without logging.
If readdirSync or readFileSync throws (e.g., permission denied), this silently drops the error. At minimum, log with context so issues are diagnosable.
Proposed fix
- } catch {}
+ } catch (err) {
+ console.warn(`[ai-chat/scanCustomCommands] Failed to read commands from ${dir}:`, err);
+ }As per coding guidelines: "Never swallow errors silently; at minimum log errors with context before rethrowing or handling them explicitly."
🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/ai-chat/index.ts` at line 51, The empty
catch after the block that calls readdirSync and readFileSync swallows
filesystem errors; replace the silent catch with explicit error handling: catch
the error object, log a descriptive message including the operation and path
(e.g., mentioning readdirSync/readFileSync and the directory/file variables
used), and then either rethrow or return a clear fallback so callers can handle
it; ensure you use the module's logger (or console.error if no logger exists)
and include the error stack/details for diagnostics.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/streams/src/sdk-to-ai-chunks.ts (1)
59-64: 🛠️ Refactor suggestion | 🟠 MajorAdd an
SDKAssistantMessageinterface to theSDKMessageunion for type safety.The
"assistant"case on line 165 matches only via the catch-all{ type: string; ... }because noSDKAssistantMessagevariant exists in the union. This means TypeScript won't narrow the type, forcing the unsafeascast insidehandleAssistantMessage. Adding a dedicated interface (consistent withSDKUserMessage,SDKResultMessage, etc.) makes the shape explicit and enables proper narrowing.Proposed fix
+interface SDKAssistantMessage { + type: "assistant"; + message?: { + content?: Array<{ type: string; text?: string }>; + }; + session_id: string; +} + type SDKMessage = | SDKPartialAssistantMessage | SDKUserMessage | SDKResultMessage | SDKSystemMessage + | SDKAssistantMessage | { type: string; [key: string]: unknown };Then update the call site:
case "assistant": - return handleAssistantMessage(state, message); + return handleAssistantMessage(state, message as SDKAssistantMessage);As per coding guidelines, "Maintain type safety by avoiding
anytypes unless absolutely necessary".Also applies to: 165-166
🤖 Fix all issues with AI agents
In `@apps/desktop/src/lib/trpc/routers/ai-chat/index.ts`:
- Around line 40-42: The front-matter regex using
raw.match(/^---\n([\s\S]*?)\n---/) fails on Windows CRLF endings; update the
raw.match pattern in the fmMatch expression to accept optional CR (use \r?\n) so
it matches both LF and CRLF, and ensure the subsequent descMatch and argMatch
(the captures from fmMatch used in descMatch and argMatch) still have their
values trimmed (they are already trimmed later) to remove any leftover \r
characters.
In `@apps/streams/src/sdk-to-ai-chunks.ts`:
- Around line 408-434: The handler currently takes a generic SDKMessage and
immediately casts it; change the function signature of handleAssistantMessage to
accept SDKAssistantMessage directly (matching handleUserMessage and
handleResultMessage) and remove the inline "as" cast and its temporary msg
variable; update any imports/types so SDKAssistantMessage defines the shape {
type: "assistant"; message?: { content?: Array<{ type: string; text?: string }>
} } and leave the rest of the function logic intact (use the typed parameter
where content is read).
🧹 Nitpick comments (2)
apps/desktop/src/lib/trpc/routers/ai-chat/index.ts (1)
65-69: Synchronous file I/O in a query handler.
scanCustomCommandsusesreaddirSync/readFileSyncwhich blocks the main process event loop. For a handful of.mdfiles this is likely fine, but if the commands directory grows or the disk is slow, it could stall the UI. Consider switching to async variants (readdir/readFilefromnode:fs/promises) if you observe any latency.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/ChatInterface.tsx (1)
281-282: TODO:handleClearis a no-op —/clearcommand will silently do nothing.The
resolveCommandActionfor "clear" setsisClear: true, which callsonClear(), but this callback is empty. Users selecting/clearfrom the menu will see the input cleared but the session won't reset.Would you like me to open an issue to track wiring this to the actual session/conversation reset logic?
| const fmMatch = raw.match(/^---\n([\s\S]*?)\n---/); | ||
| const descMatch = fmMatch?.[1]?.match(/^description:\s*(.+)$/m); | ||
| const argMatch = fmMatch?.[1]?.match(/^argument-hint:\s*(.+)$/m); |
There was a problem hiding this comment.
Front-matter regex won't match on Windows due to \r\n line endings.
readFileSync preserves OS-native line endings. On Windows, ^---\n won't match ---\r\n, so description and argumentHint will silently fall back to "" for all commands.
Proposed fix — use `\r?\n` in the regex
- const fmMatch = raw.match(/^---\n([\s\S]*?)\n---/);
- const descMatch = fmMatch?.[1]?.match(/^description:\s*(.+)$/m);
- const argMatch = fmMatch?.[1]?.match(/^argument-hint:\s*(.+)$/m);
+ const fmMatch = raw.match(/^---\r?\n([\s\S]*?)\r?\n---/);
+ const descMatch = fmMatch?.[1]?.match(/^description:\s*(.+)$/m);
+ const argMatch = fmMatch?.[1]?.match(/^argument-hint:\s*(.+)$/m);Note: the $/m flag in the inner regexes will match before \n, but .+ will greedily consume a trailing \r. You may want to .trim() the captures (already done on lines 45-46, so those are fine).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const fmMatch = raw.match(/^---\n([\s\S]*?)\n---/); | |
| const descMatch = fmMatch?.[1]?.match(/^description:\s*(.+)$/m); | |
| const argMatch = fmMatch?.[1]?.match(/^argument-hint:\s*(.+)$/m); | |
| const fmMatch = raw.match(/^---\r?\n([\s\S]*?)\r?\n---/); | |
| const descMatch = fmMatch?.[1]?.match(/^description:\s*(.+)$/m); | |
| const argMatch = fmMatch?.[1]?.match(/^argument-hint:\s*(.+)$/m); |
🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/ai-chat/index.ts` around lines 40 - 42, The
front-matter regex using raw.match(/^---\n([\s\S]*?)\n---/) fails on Windows
CRLF endings; update the raw.match pattern in the fmMatch expression to accept
optional CR (use \r?\n) so it matches both LF and CRLF, and ensure the
subsequent descMatch and argMatch (the captures from fmMatch used in descMatch
and argMatch) still have their values trimmed (they are already trimmed later)
to remove any leftover \r characters.
| function handleAssistantMessage( | ||
| state: ConversionState, | ||
| message: SDKMessage, | ||
| ): StreamChunk[] { | ||
| const msg = message as { | ||
| type: "assistant"; | ||
| message?: { content?: Array<{ type: string; text?: string }> }; | ||
| }; | ||
| const content = msg.message?.content; | ||
| if (!content || !Array.isArray(content)) return []; | ||
|
|
||
| const now = Date.now(); | ||
| const chunks: StreamChunk[] = []; | ||
|
|
||
| for (const block of content) { | ||
| if (block.type === "text" && block.text) { | ||
| chunks.push({ | ||
| type: "TEXT_MESSAGE_CONTENT", | ||
| messageId: state.messageId, | ||
| delta: block.text, | ||
| timestamp: now, | ||
| } satisfies StreamChunk); | ||
| } | ||
| } | ||
|
|
||
| return chunks; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Tighten the parameter type and align with the pattern used by sibling handlers.
handleAssistantMessage accepts SDKMessage then immediately casts it, unlike handleUserMessage(message: SDKUserMessage) and handleResultMessage(state, message: SDKResultMessage). If you add the SDKAssistantMessage interface suggested above, the signature becomes self-documenting and the as cast on lines 412-415 can be removed entirely.
Proposed fix (assumes SDKAssistantMessage exists)
function handleAssistantMessage(
state: ConversionState,
- message: SDKMessage,
+ message: SDKAssistantMessage,
): StreamChunk[] {
- const msg = message as {
- type: "assistant";
- message?: { content?: Array<{ type: string; text?: string }> };
- };
- const content = msg.message?.content;
+ const content = message.message?.content;
if (!content || !Array.isArray(content)) return [];🤖 Prompt for AI Agents
In `@apps/streams/src/sdk-to-ai-chunks.ts` around lines 408 - 434, The handler
currently takes a generic SDKMessage and immediately casts it; change the
function signature of handleAssistantMessage to accept SDKAssistantMessage
directly (matching handleUserMessage and handleResultMessage) and remove the
inline "as" cast and its temporary msg variable; update any imports/types so
SDKAssistantMessage defines the shape { type: "assistant"; message?: { content?:
Array<{ type: string; text?: string }> } } and leave the rest of the function
logic intact (use the typed parameter where content is read).
… /clear case Emit a TEXT_MESSAGE_CONTENT chunk before RUN_ERROR so error messages appear in the chat bubble (RUN_ERROR alone is invisible in the materialization pipeline). Also remove the dead /clear branch from resolveCommandAction and its associated onClear plumbing.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/streams/src/sdk-to-ai-chunks.ts (2)
59-64: 🛠️ Refactor suggestion | 🟠 MajorAdd
SDKAssistantMessageto theSDKMessageunion for proper type narrowing.The
"assistant"case at Line 165 matches only via the catch-all{ type: string; [key: string]: unknown }, so TypeScript cannot narrow the type. This is the root cause of the unsafeascast inhandleAssistantMessage. Adding an explicit interface to the union enables the switch to narrow correctly and eliminates the need for the cast downstream.Proposed fix
+interface SDKAssistantMessage { + type: "assistant"; + message?: { content?: Array<{ type: string; text?: string }> }; + session_id?: string; +} + type SDKMessage = | SDKPartialAssistantMessage | SDKUserMessage | SDKResultMessage | SDKSystemMessage + | SDKAssistantMessage | { type: string; [key: string]: unknown };Then in
convertMessage:case "assistant": - return handleAssistantMessage(state, message); + return handleAssistantMessage(state, message as SDKAssistantMessage);And tighten the handler signature (see past review comment):
-function handleAssistantMessage( - state: ConversionState, - message: SDKMessage, -): StreamChunk[] { - const msg = message as { - type: "assistant"; - message?: { content?: Array<{ type: string; text?: string }> }; - }; - const content = msg.message?.content; +function handleAssistantMessage( + state: ConversionState, + message: SDKAssistantMessage, +): StreamChunk[] { + const content = message.message?.content;Also applies to: 165-166
447-463:⚠️ Potential issue | 🟡 MinorGood approach making errors visible in chat; minor redundancy in the displayed text.
Emitting a
TEXT_MESSAGE_CONTENTchunk beforeRUN_ERRORis a solid fix for the invisible-error problem.However, since
message.subtypealready starts with"error"(per the guard on Line 447), the user-facing delta becomes something like"Error: error_timeout"— doubling the word "error." Consider stripping the prefix or using a cleaner format:Proposed fix
chunks.push({ type: "TEXT_MESSAGE_CONTENT", messageId: state.messageId, - delta: `Error: ${message.subtype}`, + delta: `Error: ${message.subtype.replace(/^error[_\s]*/i, "")}`, timestamp: now, } satisfies StreamChunk);
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/hooks/useSlashCommands/useSlashCommands.ts (1)
62-66:navigateUpsetsselectedIndexto-1whenfilteredCommandsis empty.If
filteredCommands.lengthis0, the expressionprev <= 0 ? filteredCommands.length - 1 : prev - 1evaluates to-1. While the returnedisOpenguards against this at the consumer level (Line 75), the functions are still exposed and could be called independently.A simple clamp or early return would make this defensive:
Suggested fix
const navigateUp = useCallback(() => { + if (filteredCommands.length === 0) return; setSelectedIndex((prev) => prev <= 0 ? filteredCommands.length - 1 : prev - 1, ); }, [filteredCommands.length]); const navigateDown = useCallback(() => { + if (filteredCommands.length === 0) return; setSelectedIndex((prev) => prev >= filteredCommands.length - 1 ? 0 : prev + 1, ); }, [filteredCommands.length]);
1 participant
Summary
/prefix detection, keyboard navigation, and command executionChanges
Chat — Slash Commands
apps/streams/src/claude-agent.ts—GET /commandsendpoint withsupportedCommands()cache + fallback defaultsapps/desktop/.../ai-chat/index.ts—getSlashCommandstRPC procedureapps/desktop/.../ChatInterface/hooks/useSlashCommands/— Hook for menu state, filtering, navigationapps/desktop/.../ChatInterface/components/SlashCommandMenu/— Popover command list componentapps/desktop/.../ChatInterface/ChatInterface.tsx—PromptInputProviderwrapper,SlashCommandInputinner component with keyboard captureChat — Tool Call UI
bash-tool,web-fetch-tool,web-search-tool,file-diff-tool,user-question-tool,exploring-group,tool-call,tool-interruptedChatMessageItem— Rich rendering for tool calls, thinking blocks, citationsToolCallBlock— Expanded tool result display with tool registryThinkingToggle— Toggle for extended thinking modeContextIndicator— Token usage displayChat — Session History
claude-session-scanner— Scan and read Claude Code session files from diskuseClaudeCodeHistory— Merge live messages with historical session dataSessionSelector— Browse and restore past sessionsChanges View
LightDiffViewer— Lightweight diff rendering componentFileDiffHeader+useFileDiffEdit— Edit mode toggle for diffsFileDiffSection— Enhanced diff section with edit supportOther
fix(desktop): Preserve file explorer state when switching sidebar tabsfeat(marketing): Geist Pixel font for hero headlinefeat(desktop): Teardown logs dialogfix(desktop): Auto-updater diagnostics and dep bumpstext-selection-popover— Text selection popover UI componentTest Plan
/in chat input — menu shows all commands/co— filters to/compact,/config,/cost/compactinserts/compactwith cursor for args/helpsends immediatelySummary by CodeRabbit
New Features
Bug Fixes