feat: Implement slash command menu with fixed positioning and dark mode #211
Conversation
## Features - **Token Budget Visualization**: Added real-time token usage tracking with pie chart display showing percentage used (blue < 50%, orange < 75%, red ≥ 75%) - **Show Thinking Toggle**: Added quick settings option to show/hide reasoning sections in messages - **Cache Clearing Utility**: Added `/clear-cache.html` page for clearing service workers, caches, and storage ## Improvements - **Package Upgrades**: Migrated from deprecated `xterm` to `@xterm/*` scoped packages - **Testing Setup**: Added Playwright for end-to-end testing - **Build Optimization**: Implemented code splitting for React, CodeMirror, and XTerm vendors to improve initial load time - **Deployment Scripts**: Added `scripts/start.sh` and `scripts/stop.sh` for cleaner server management with automatic port conflict resolution - **Vite Update**: Upgraded Vite from 7.0.5 to 7.1.8 ## Bug Fixes - Fixed static file serving to properly handle routes vs assets - Fixed session state reset to preserve token budget on initial load - Updated default Vite dev server port to 5173 (Vite's standard) ## Technical Details - Token budget is parsed from Claude CLI `modelUsage` field in result messages - Budget updates are sent via WebSocket as `token-budget` events - Calculation includes input, output, cache read, and cache creation tokens - Token budget state persists during active sessions but resets on session switch
Fixes issue where "Thinking..." banner and stop button disappear when switching between sessions. Users can now navigate freely while Claude is processing without losing the ability to monitor or stop the session. Features: - Processing state tracked in processingSessions Set (App.jsx) - Backend session status queries via check-session-status WebSocket message - UI state (banner + stop button) restored when returning to processing sessions - Works after page reload by querying backend's authoritative process maps - Proper cleanup when sessions complete in background Backend Changes: - Added sessionId to claude-complete, cursor-result, session-aborted messages - Exported isClaudeSessionActive, isCursorSessionActive helper functions - Exported getActiveClaudeSessions, getActiveCursorSessions for status queries - Added check-session-status and get-active-sessions WebSocket handlers Frontend Changes: - processingSessions state tracking in App.jsx - onSessionProcessing/onSessionNotProcessing callbacks - Session status check on session load and switch - Completion handlers only update UI if message is for current session - Always clean up processing state regardless of which session is active
Removes hardcoded 160k token limit and makes it configurable through environment variables. This allows easier adjustment for different Claude models or use cases. Changes: - Added CONTEXT_WINDOW env var for backend (default: 160000) - Added VITE_CONTEXT_WINDOW env var for frontend (default: 160000) - Updated .env.example with documentation - Replaced hardcoded values in token usage calculations - Replaced hardcoded values in pie chart display Why 160k? Claude Code reserves ~40k tokens for auto-compact feature, leaving 160k available for actual usage from the 200k context window.
HTML entities like < and > were showing as-is instead of being decoded to < and > characters. Added decodeHtmlEntities helper function to properly display angle brackets and other special characters. Applied to: - Regular message content - Streaming content deltas - Session history loading - Both string and array content types
- Revert to main branch's package.json scripts structure - Remove custom scripts/start.sh and scripts/stop.sh - Update xterm dependencies to scoped @xterm packages (required for code compatibility) - Replace xterm with @xterm/xterm - Replace xterm-addon-fit with @xterm/addon-fit 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit completes the migration to the Claude Agents SDK, removing the legacy CLI-based implementation and making the SDK the exclusive integration method. Changes: - Remove claude-cli.js legacy implementation - Add claude-sdk.js with full SDK integration - Remove CLAUDE_USE_SDK feature flag (SDK is now always used) - Update server/index.js to use SDK functions directly - Add .serena/ to .gitignore for AI assistant cache Benefits: - Better performance (no child process overhead) - Native session management with interrupt support - Cleaner codebase without CLI/SDK branching - Full feature parity with previous CLI implementation - Maintains compatibility with Cursor integration 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Whoops. This is correct. Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Left my test code in, but that's fixed. Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…itch - Add AbortController to cancel in-flight token-usage requests when session/project changes - Capture session/project IDs before fetch and verify they match before updating state - Handle AbortError gracefully without logging as error - Prevents race condition where old session data overwrites current session's token budget 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…de support - Add CommandMenu component with grouped command display - Implement command routes for listing, loading, and executing commands - Add command parser utility for argument and file processing - Fix menu positioning using fixed positioning relative to viewport - Add dark mode support with proper text contrast - Preserve metadata badge colors in dark mode - Support built-in, project, and user-level commands - Add keyboard navigation and selection 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughThe PR migrates Claude CLI-based execution to Claude Agents SDK integration, introduces a slash-command system with custom command parsing and bash execution, adds token usage tracking and display, enhances session state management with processing indicators, updates xterm dependencies to scoped packages, and adds cache-clearing utilities. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Frontend as Frontend<br/>(ChatInterface)
participant WS as WebSocket
participant Server as Server<br/>(index.js)
participant SDK as Claude SDK<br/>(claude-sdk.js)
participant Claude as Claude Agent
rect rgb(200, 220, 255)
Note over Frontend,Claude: Old Flow (Removed): CLI-based
Frontend->>Server: /api/projects/.../chat<br/>(Claude command)
Server->>Server: spawnClaude (CLI process)
Server->>Claude: spawn CLI process
Claude-->>Server: stdout stream
Server->>WS: claude-response message
WS->>Frontend: display in chat
Server->>Server: cleanup process
end
rect rgb(220, 255, 220)
Note over Frontend,Claude: New Flow (Added): SDK-based
Frontend->>Server: /api/projects/.../chat<br/>(Claude command)
Server->>SDK: queryClaudeSDK()
SDK->>SDK: mapCliOptionsToSDK()
SDK->>SDK: loadMcpConfig()
SDK->>SDK: handleImages()
SDK->>Claude: SDK query
Claude-->>SDK: stream message
SDK->>WS: transformMessage()
WS->>Frontend: WebSocket payload
SDK->>SDK: extractTokenBudget()
SDK->>WS: token-budget event
SDK->>SDK: cleanup temp files
SDK->>WS: claude-complete
end
rect rgb(255, 235, 200)
Note over Frontend,Server: Command Execution (New)
Frontend->>Server: /api/commands/execute
Server->>Server: load command file
Server->>Server: replaceArguments()
Server->>Server: processFileIncludes()
Server->>Server: processBashCommands()
Server-->>Frontend: command content + metadata
end
rect rgb(255, 220, 235)
Note over Frontend,Server: Token Usage Polling (New)
Frontend->>Frontend: periodic timer
Frontend->>Server: GET /api/.../token-usage
Server-->>Frontend: { used, total, contextWindow }
Frontend->>Frontend: update TokenUsagePie
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Rationale: The PR introduces substantial heterogeneous changes spanning multiple domains: (1) architectural migration from CLI to SDK with session/streaming management, (2) new command system with markdown parsing and bash execution involving security considerations, (3) frontend state management enhancements for processing tracking, and (4) new polling/display logic for token usage. While individual file changes may be moderate, the interconnected nature of these systems—token budget extraction tied to streaming, session management affecting multiple APIs, command execution with security validation—demands careful reasoning across each domain to ensure correctness of integration points, error handling, and state consistency. Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/ChatInterface.jsx (3)
2396-2555: Wrap case 'claude-response' declarations.Fix lint by scoping consts within a block.
Apply:
- case 'claude-response': - const messageData = latestMessage.data.message || latestMessage.data; + case 'claude-response': { + const messageData = latestMessage.data.message || latestMessage.data; // ... existing logic unchanged ... + break; + }Note: Keep existing early returns; the added break covers the non-return path.
2682-2744: Wrap case 'cursor-result' declarations.Scope consts and add break.
Apply:
- case 'cursor-result': - // Get session ID from message or fall back to current session - const cursorCompletedSessionId = latestMessage.sessionId || currentSessionId; + case 'cursor-result': { + const cursorCompletedSessionId = latestMessage.sessionId || currentSessionId; // ... existing logic ... - const pendingCursorSessionId = sessionStorage.getItem('pendingSessionId'); + const pendingCursorSessionId = sessionStorage.getItem('pendingSessionId'); // ... existing logic ... - break; + break; + }
2778-2814: Wrap case 'claude-complete' declarations.Scope consts and add break.
Apply:
- case 'claude-complete': - // Get session ID from message or fall back to current session - const completedSessionId = latestMessage.sessionId || currentSessionId || sessionStorage.getItem('pendingSessionId'); + case 'claude-complete': { + const completedSessionId = latestMessage.sessionId || currentSessionId || sessionStorage.getItem('pendingSessionId'); // ... existing logic ... - break; + break; + }
🧹 Nitpick comments (9)
.env.example (1)
17-20: Consider reordering environment variables for consistency.The static analysis tool suggests placing
CONTEXT_WINDOWbeforeVITE_CONTEXT_WINDOWto maintain alphabetical ordering within the section. This is a minor consistency improvement.Apply this diff to reorder the variables:
# Claude Code context window size (maximum tokens per session) # Note: VITE_ prefix makes it available to frontend -VITE_CONTEXT_WINDOW=160000 CONTEXT_WINDOW=160000 +VITE_CONTEXT_WINDOW=160000public/clear-cache.html (1)
55-77: Consider using textContent or DOM methods instead of innerHTML concatenation.While the current implementation is safe (no user input or external data), repeatedly concatenating to
innerHTMLcauses the browser to re-parse the entire HTML on each append, which can impact performance. Consider usinginsertAdjacentHTML()or creating DOM elements programmatically for better performance.For example, you could refactor to use
insertAdjacentHTML:- status.innerHTML += '<p class="success">✓ Unregistered service worker</p>'; + status.insertAdjacentHTML('beforeend', '<p class="success">✓ Unregistered service worker</p>');Or create elements programmatically:
- status.innerHTML += '<p class="success">✓ Unregistered service worker</p>'; + const p = document.createElement('p'); + p.className = 'success'; + p.textContent = '✓ Unregistered service worker'; + status.appendChild(p);server/utils/commandParser.js (2)
100-134: Preserve leading whitespace/newlines for @includes.Replacement keeps only a single space; it drops tabs/newlines, breaking formatting. Preserve the actual leading char.
Apply:
- // Replace the @filename with the file content - result = result.replace(fullMatch, fullMatch.startsWith(' ') ? ' ' + processedContent : processedContent); + // Replace the @filename with the file content, preserving any leading whitespace/newline + const leading = fullMatch[0] === '@' ? '' : fullMatch[0]; + result = result.replace(fullMatch, leading + processedContent);
181-186: Improve bang-command detection to allow leading spaces.Current regex misses lines like " !cmd". Consider allowing leading whitespace.
Apply:
- const commandPattern = /(?:^|\n)!(.+?)(?=\n|$)/g; + const commandPattern = /(?:^|\n)\s*!(.+?)(?=\n|$)/g;server/routes/commands.js (2)
518-530: Reuse replaceArguments from utils to avoid divergence.Duplicate logic risks inconsistencies with parser behavior. Import and use replaceArguments(content, args).
Apply:
- // Basic argument replacement (will be enhanced in command parser utility) - let processedContent = commandContent; - // Replace $ARGUMENTS with all arguments joined - const argsString = args.join(' '); - processedContent = processedContent.replace(/\$ARGUMENTS/g, argsString); - // Replace $1, $2, etc. with positional arguments - args.forEach((arg, index) => { - const placeholder = `$${index + 1}`; - processedContent = processedContent.replace(new RegExp(`\\${placeholder}\\b`, 'g'), arg); - }); + // Argument replacement using shared utility + let processedContent = commandContent; + const { replaceArguments } = await import('../utils/commandParser.js'); + processedContent = replaceArguments(processedContent, args);
536-538: Use anchored regexes for include/bash flags.String includes('!') is noisy. Prefer line-anchored checks.
Apply:
- hasFileIncludes: processedContent.includes('@'), - hasBashCommands: processedContent.includes('!') + hasFileIncludes: /(^|\n)\s*@\S+/.test(processedContent), + hasBashCommands: /(^|\n)\s*!\S+/.test(processedContent)server/claude-sdk.js (1)
376-389: Guard WebSocket sends.Avoid throws if socket closed; check readyState before send.
Apply pattern at each send site:
- ws.send(JSON.stringify({ + if (ws && ws.readyState === 1) ws.send(JSON.stringify({ type: 'session-created', sessionId: capturedSessionId - })); + }));Repeat for other ws.send calls in this module.
Also applies to: 395-399, 412-417, 431-435
src/components/ChatInterface.jsx (2)
4072-4092: Unify click/keyboard selection behavior (update input + execute).CommandMenu onSelect currently executes without updating input. Route clicks through selectCommand for consistency.
Apply:
- <CommandMenu - commands={filteredCommands} - selectedIndex={selectedCommandIndex} - onSelect={handleCommandSelect} + <CommandMenu + commands={filteredCommands} + selectedIndex={selectedCommandIndex} + onSelect={(cmd, idx, isHover) => { + if (isHover) { + setSelectedCommandIndex(idx); + } else { + selectCommand(cmd); + } + }}
4083-4090: Clamp menu Y position to viewport.top can be negative when the textarea is near the top. Clamp to a small margin.
Apply:
position={{ - top: textareaRef.current - ? textareaRef.current.getBoundingClientRect().top - 308 - : 0, + top: (() => { + const t = textareaRef.current?.getBoundingClientRect().top ?? 0; + return Math.max(8, t - 308); + })(), left: textareaRef.current ? textareaRef.current.getBoundingClientRect().left : 0 }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (18)
.env.example(1 hunks).gitignore(2 hunks)package.json(1 hunks)public/clear-cache.html(1 hunks)server/claude-cli.js(0 hunks)server/claude-sdk.js(1 hunks)server/cursor-cli.js(3 hunks)server/index.js(7 hunks)server/routes/commands.js(1 hunks)server/utils/commandParser.js(1 hunks)src/App.jsx(5 hunks)src/components/ChatInterface.jsx(30 hunks)src/components/CommandMenu.jsx(1 hunks)src/components/MainContent.jsx(2 hunks)src/components/QuickSettingsPanel.jsx(2 hunks)src/components/Shell.jsx(1 hunks)src/components/TokenUsagePie.jsx(1 hunks)vite.config.js(1 hunks)
💤 Files with no reviewable changes (1)
- server/claude-cli.js
🧰 Additional context used
🧬 Code graph analysis (9)
server/cursor-cli.js (2)
server/claude-sdk.js (1)
capturedSessionId(334-334)server/routes/commands.js (1)
sessionId(221-221)
src/components/CommandMenu.jsx (1)
src/components/ChatInterface.jsx (1)
frequentCommands(1371-1397)
src/components/MainContent.jsx (1)
src/App.jsx (2)
processingSessions(71-71)showThinking(60-60)
server/routes/commands.js (2)
server/index.js (10)
__filename(8-8)__dirname(9-9)path(1016-1016)fs(1017-1017)content(384-384)homeDir(315-315)homeDir(1110-1110)os(1018-1018)result(227-227)result(244-244)server/utils/commandParser.js (6)
process(178-178)resolvedPath(80-80)result(54-54)result(108-108)result(188-188)processedContent(124-124)
server/utils/commandParser.js (2)
server/routes/mcp.js (1)
exec(22-22)server/routes/commands.js (6)
content(39-39)content(448-448)content(515-515)result(492-492)resolvedPath(438-438)processedContent(519-519)
server/claude-sdk.js (4)
src/components/ChatInterface.jsx (1)
permissionMode(1209-1209)server/routes/commands.js (1)
sessionId(221-221)src/components/Sidebar.jsx (1)
getAllSessions(336-343)server/index.js (7)
inputTokens(1125-1125)cacheReadTokens(1127-1127)cacheCreationTokens(1126-1126)totalUsed(1152-1152)contextWindow(1124-1124)i(1130-1130)os(1018-1018)
src/components/QuickSettingsPanel.jsx (1)
src/App.jsx (1)
showThinking(60-60)
src/components/ChatInterface.jsx (3)
src/App.jsx (5)
showThinking(60-60)selectedProject(49-49)selectedSession(50-50)processingSessions(71-71)sendByCtrlEnter(62-62)src/utils/api.js (3)
authenticatedFetch(2-20)authenticatedFetch(2-20)url(53-53)src/components/CommandMenu.jsx (1)
CommandMenu(13-290)
server/index.js (3)
server/routes/commands.js (1)
sessionId(221-221)server/claude-sdk.js (6)
activeSessions(21-21)contextWindow(170-170)inputTokens(163-163)cacheCreationTokens(166-166)cacheReadTokens(165-165)totalUsed(169-169)server/cursor-cli.js (1)
process(239-239)
🪛 Biome (2.1.2)
server/utils/commandParser.js
[error] 166-166: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 166-166: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 166-166: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 166-166: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 166-166: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 166-166: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
src/components/ChatInterface.jsx
[error] 1525-1526: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 1535-1536: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2397-2399: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2682-2684: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2734-2735: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2779-2781: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2818-2820: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2847-2847: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2847-2849: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 20-20: [UnorderedKey] The CONTEXT_WINDOW key should go before the VITE_CONTEXT_WINDOW key
(UnorderedKey)
🔇 Additional comments (8)
.gitignore (2)
108-108: LGTM!The addition of
.serena/to the AI-specific ignore section is consistent with the existing pattern for AI tool directories.
130-131: LGTM!Re-enabling the ignore rules for
tasks.jsonandtasks/is appropriate to prevent task-related files from being committed to version control.server/cursor-cli.js (2)
162-162: LGTM!The fallback to
sessionIdwhencapturedSessionIdis not available ensures correct session tracking in both new and resumed session scenarios.Also applies to: 205-205
249-256: LGTM!The new session query utilities provide a clean interface for checking active Cursor sessions and align with similar functions in
server/claude-sdk.js.src/components/Shell.jsx (1)
2-6: LGTM!The migration to scoped
@xtermpackages is correctly implemented, with proper import paths for the core library, addons, and CSS. This aligns with the dependency changes inpackage.json.Based on learnings
vite.config.js (1)
27-45: LGTM!The build optimization with manual chunking is well-structured:
- Separates vendor libraries into logical groups (React, CodeMirror, xterm)
- The 1000 KB chunk size warning limit is reasonable
- The xterm chunk correctly includes all
@xtermscoped packages, aligning with the dependency migrationThis improves caching efficiency and enables parallel loading of vendor bundles.
src/components/QuickSettingsPanel.jsx (1)
132-143: LGTM!The "Show thinking" toggle is well-implemented and follows the same pattern as other settings in the component. The use of the Brain icon is semantically appropriate, and the checkbox includes proper dark mode support and accessibility features.
package.json (1)
76-77: No action required:@xterm/addon-fit@0.10.0declares a peer dependency of@xterm/xterm: ^5.0.0, which covers version 5.5.0.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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 (4)
server/index.js (4)
379-386: Constrain file reads to the project root (prevent cross-project/system reads).This endpoint accepts any absolute path; an authenticated user could read arbitrary files. Ensure the path resolves under the project’s directory.
- // Security check - ensure the path is safe and absolute - if (!filePath || !path.isAbsolute(filePath)) { - return res.status(400).json({ error: 'Invalid file path' }); - } + // Security: ensure the requested path is inside the project root + if (!filePath) { + return res.status(400).json({ error: 'Invalid file path' }); + } + const projectRoot = await extractProjectDirectory(projectName).catch(() => null); + if (!projectRoot) { + return res.status(404).json({ error: 'Project not found' }); + } + const resolved = path.resolve(filePath); + const normalizedRoot = path.resolve(projectRoot) + path.sep; + if (!resolved.startsWith(normalizedRoot)) { + return res.status(403).json({ error: 'Path must be under project root' }); + }Apply the same guard to the binary content and save endpoints below.
409-419: Apply project-root guard here as well (binary content).Mirror the project root check to prevent serving arbitrary system files.
- // Security check - ensure the path is safe and absolute - if (!filePath || !path.isAbsolute(filePath)) { - return res.status(400).json({ error: 'Invalid file path' }); - } + if (!filePath) { + return res.status(400).json({ error: 'Invalid file path' }); + } + const projectRoot = await extractProjectDirectory(projectName).catch(() => null); + if (!projectRoot) { + return res.status(404).json({ error: 'Project not found' }); + } + const resolved = path.resolve(filePath); + const normalizedRoot = path.resolve(projectRoot) + path.sep; + if (!resolved.startsWith(normalizedRoot)) { + return res.status(403).json({ error: 'Path must be under project root' }); + }
454-461: Constrain writes to the project root (prevent arbitrary writes).Ensure the save target resides under the project directory before writing.
- // Security check - ensure the path is safe and absolute - if (!filePath || !path.isAbsolute(filePath)) { - return res.status(400).json({ error: 'Invalid file path' }); - } + if (!filePath) { + return res.status(400).json({ error: 'Invalid file path' }); + } + const projectRoot = await extractProjectDirectory(projectName).catch(() => null); + if (!projectRoot) { + return res.status(404).json({ error: 'Project not found' }); + } + const resolved = path.resolve(filePath); + const normalizedRoot = path.resolve(projectRoot) + path.sep; + if (!resolved.startsWith(normalizedRoot)) { + return res.status(403).json({ error: 'Path must be under project root' }); + }
110-114: Use WebSocket.OPEN instead of client.OPEN – importWebSocketfrom 'ws' and changeclient.readyState === client.OPENtoclient.readyState === WebSocket.OPEN.
♻️ Duplicate comments (1)
server/index.js (1)
1111-1113: Windows-safe projectPath encoding fix confirmed.Including backslashes and colons prevents accidental absolute paths on Windows. Nice.
🧹 Nitpick comments (4)
server/index.js (4)
577-585: Validate abort payload before acting.If sessionId is missing/empty, abort handlers may misbehave. Add a quick guard and error response.
} else if (data.type === 'abort-session') { console.log('🛑 Abort session request:', data.sessionId); const provider = data.provider || 'claude'; - let success; + if (!data.sessionId) { + return ws.send(JSON.stringify({ + type: 'session-aborted', + sessionId: data.sessionId, + provider, + success: false, + error: 'sessionId is required' + })); + } + let success;
601-619: Guard check-session-status inputs.Return a clear error when sessionId is missing; defaulting to undefined can be confusing.
} else if (data.type === 'check-session-status') { // Check if a specific session is currently processing const provider = data.provider || 'claude'; const sessionId = data.sessionId; + if (!sessionId) { + return ws.send(JSON.stringify({ + type: 'session-status', + sessionId, + provider, + isProcessing: false, + error: 'sessionId is required' + })); + } let isActive;
60-61: Home directory resolution: prefer os.homedir().process.env.HOME may be unset on Windows. Use os.homedir() for cross‑platform reliability.
- const claudeProjectsPath = path.join(process.env.HOME, '.claude', 'projects'); + const claudeProjectsPath = path.join(os.homedir(), '.claude', 'projects');
514-516: Remove unused variable.hiddenFiles is computed but never used.
- const hiddenFiles = files.filter(f => f.name.startsWith('.')); res.json(files);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
server/index.js(7 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
server/index.js (5)
server/middleware/auth.js (1)
authenticateToken(22-45)server/routes/commands.js (3)
sessionId(221-221)homeDir(394-394)__dirname(9-9)server/claude-sdk.js (5)
activeSessions(21-21)inputTokens(163-163)cacheCreationTokens(166-166)cacheReadTokens(165-165)totalUsed(169-169)server/projects.js (4)
lines(815-815)entry(300-300)entry(663-663)entry(738-738)server/cursor-cli.js (1)
process(239-239)
🔇 Additional comments (5)
server/index.js (5)
196-198: Commands API route wiring looks good.Route is properly protected and registered after auth.
558-560: Good switch to Claude Agents SDK.Directly routing claude-command to queryClaudeSDK keeps the WS flow cohesive.
621-629: Active sessions aggregation looks solid.Nice consolidation of Claude and Cursor active sets.
1171-1178: Dev redirect LGTM.Honors VITE_PORT with sensible default.
1271-1273: Clear runtime log message.Good visibility that Claude Agents SDK is active.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/ChatInterface.jsx (1)
2396-2560: Wrap 'claude-response' case to scope declarations.messageData (and other consts) should be block-scoped per case.
Apply:
- case 'claude-response': - const messageData = latestMessage.data.message || latestMessage.data; + case 'claude-response': { + const messageData = latestMessage.data.message || latestMessage.data; @@ - break; + break; + }
♻️ Duplicate comments (7)
src/components/ChatInterface.jsx (4)
2681-2744: Wrap 'cursor-result' case to scope declarations.Block-scope cursorCompletedSessionId and locals.
Apply:
- case 'cursor-result': - // Get session ID from message or fall back to current session - const cursorCompletedSessionId = latestMessage.sessionId || currentSessionId; + case 'cursor-result': { + const cursorCompletedSessionId = latestMessage.sessionId || currentSessionId; @@ - break; + break; + }Based on learnings
2779-2815: Wrap 'claude-complete' case to scope declarations.Block-scope completedSessionId and pendingSessionId.
Apply:
- case 'claude-complete': - // Get session ID from message or fall back to current session - const completedSessionId = latestMessage.sessionId || currentSessionId || sessionStorage.getItem('pendingSessionId'); + case 'claude-complete': { + const completedSessionId = latestMessage.sessionId || currentSessionId || sessionStorage.getItem('pendingSessionId'); @@ - break; + break; + }Based on learnings
1523-1542: Wrap switch-case declarations (noSwitchDeclarations).Block-scope consts inside 'cost' and 'status' cases to satisfy linter and avoid leakage.
Apply:
- case 'cost': - // Show cost information - const costMessage = `**Token Usage**: ${data.tokenUsage.used.toLocaleString()} / ${data.tokenUsage.total.toLocaleString()} (${data.tokenUsage.percentage}%)\n\n**Estimated Cost**:\n- Input: $${data.cost.input}\n- Output: $${data.cost.output}\n- **Total**: $${data.cost.total}\n\n**Model**: ${data.model}`; - setChatMessages(prev => [...prev, { - role: 'assistant', - content: costMessage, - timestamp: Date.now() - }]); - break; + case 'cost': { + const costMessage = `**Token Usage**: ${data.tokenUsage.used.toLocaleString()} / ${data.tokenUsage.total.toLocaleString()} (${data.tokenUsage.percentage}%)\n\n**Estimated Cost**:\n- Input: $${data.cost.input}\n- Output: $${data.cost.output}\n- **Total**: $${data.cost.total}\n\n**Model**: ${data.model}`; + setChatMessages(prev => [...prev, { role: 'assistant', content: costMessage, timestamp: Date.now() }]); + break; + } @@ - case 'status': - // Show status information - const statusMessage = `**System Status**\n\n- Version: ${data.version}\n- Uptime: ${data.uptime}\n- Model: ${data.model}\n- Provider: ${data.provider}\n- Node.js: ${data.nodeVersion}\n- Platform: ${data.platform}`; - setChatMessages(prev => [...prev, { - role: 'assistant', - content: statusMessage, - timestamp: Date.now() - }]); - break; + case 'status': { + const statusMessage = `**System Status**\n\n- Version: ${data.version}\n- Uptime: ${data.uptime}\n- Model: ${data.model}\n- Provider: ${data.provider}\n- Node.js: ${data.nodeVersion}\n- Platform: ${data.platform}`; + setChatMessages(prev => [...prev, { role: 'assistant', content: statusMessage, timestamp: Date.now() }]); + break; + }Based on learnings
2816-2843: Wrap 'session-aborted' case to scope declarations.Block-scope abortedSessionId and locals.
Apply:
- case 'session-aborted': - // Get session ID from message or fall back to current session - const abortedSessionId = latestMessage.sessionId || currentSessionId; + case 'session-aborted': { + const abortedSessionId = latestMessage.sessionId || currentSessionId; @@ - break; + break; + }Based on learnings
server/utils/commandParser.js (3)
79-83: Fix directory traversal check (startsWith is unsafe).startsWith allows siblings like "/base-dir-evil". Use path.relative and ensure it doesn’t escape base.
Apply:
export function isPathSafe(filePath, basePath) { - const resolvedPath = path.resolve(basePath, filePath); - const resolvedBase = path.resolve(basePath); - return resolvedPath.startsWith(resolvedBase); + const resolvedPath = path.resolve(basePath, filePath); + const resolvedBase = path.resolve(basePath); + const relative = path.relative(resolvedBase, resolvedPath); + return ( + relative !== '' && + !relative.startsWith('..') && + !path.isAbsolute(relative) + ); }Based on learnings
144-155: Harden allowlist matching (no prefixes) and align with non-shell exec.Validate exact command name, not prefixes or path prefixes.
Apply:
export function isBashCommandAllowed(command) { const trimmedCommand = command.trim(); if (!trimmedCommand) return false; const firstWord = trimmedCommand.split(/\s+/)[0]; - return BASH_COMMAND_ALLOWLIST.some(allowed => - firstWord === allowed || firstWord.startsWith(allowed + '/') - ); + return BASH_COMMAND_ALLOWLIST.includes(firstWord); }Based on learnings
4-8: Shell injection risk: exec with Bash; execute without a shell + reject metacharacters.Current exec runs via /bin/bash and interprets metacharacters; combine with loose allowlist it’s exploitable. Switch to execFile, parse args, and reject shell metacharacters.
Apply:
-import { exec } from 'child_process'; +import { execFile } from 'child_process'; import { promisify } from 'util'; -const execAsync = promisify(exec); +const execFileAsync = promisify(execFile); @@ - const { stdout, stderr } = await execAsync(command, { - cwd, - timeout, - maxBuffer: 1024 * 1024, // 1MB max output - shell: '/bin/bash' - }); + // Reject common shell metacharacters outright + if (/[;&|`$<>]/.test(command)) { + throw new Error('Command contains disallowed shell characters'); + } + // Basic split; for quotes support, adopt a shell-words parser + const [cmd, ...args] = command.split(/\s+/); + const { stdout, stderr } = await execFileAsync(cmd, args, { + cwd, + timeout, + maxBuffer: 1024 * 1024 + });Optional: replace naive split with a proper shell-quote parser. Based on learnings
Also applies to: 207-213
🧹 Nitpick comments (2)
server/index.js (1)
1114-1121: Avoid sync I/O on request path; use async fs APIs.existsSync/readFileSync block the event loop. Use fsPromises.stat/readFile with await.
Apply:
- if (!fs.existsSync(jsonlPath)) { - return res.status(404).json({ error: 'Session file not found', path: jsonlPath }); - } + try { + await fsPromises.stat(jsonlPath); + } catch { + return res.status(404).json({ error: 'Session file not found', path: jsonlPath }); + } @@ - const fileContent = fs.readFileSync(jsonlPath, 'utf8'); + const fileContent = await fsPromises.readFile(jsonlPath, 'utf8');src/components/ChatInterface.jsx (1)
1428-1436: Escape regex when parsing args for selected command.command.name may contain regex metacharacters; use escapeRegExp to avoid incorrect matches.
Apply:
- const commandMatch = input.match(new RegExp(`${command.name}\\s*(.*)`)); + const commandMatch = input.match(new RegExp(`${escapeRegExp(command.name)}\\s*(.*)`));Add helper near the top of the file:
function escapeRegExp(str) { return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
server/index.js(7 hunks)server/utils/commandParser.js(1 hunks)src/components/ChatInterface.jsx(30 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
server/index.js (3)
server/routes/commands.js (3)
sessionId(221-221)homeDir(394-394)__dirname(9-9)server/claude-sdk.js (6)
activeSessions(21-21)contextWindow(170-170)inputTokens(163-163)cacheCreationTokens(166-166)cacheReadTokens(165-165)totalUsed(169-169)server/cursor-cli.js (1)
process(239-239)
src/components/ChatInterface.jsx (4)
src/App.jsx (5)
showThinking(60-60)selectedProject(49-49)selectedSession(50-50)processingSessions(71-71)sendByCtrlEnter(62-62)src/utils/api.js (3)
authenticatedFetch(2-20)authenticatedFetch(2-20)url(53-53)server/routes/commands.js (5)
allCommands(380-380)result(492-492)content(39-39)content(448-448)content(515-515)src/components/CommandMenu.jsx (1)
CommandMenu(13-290)
server/utils/commandParser.js (1)
server/routes/commands.js (6)
content(39-39)content(448-448)content(515-515)result(492-492)resolvedPath(438-438)processedContent(519-519)
🪛 Biome (2.1.2)
src/components/ChatInterface.jsx
[error] 1525-1526: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 1535-1536: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2397-2399: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2682-2684: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2734-2735: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2779-2781: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2818-2820: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (1)
src/components/ChatInterface.jsx (1)
3415-3417: Nice fix: removed duplicate leading slash on insert.Using command.name directly avoids '//' insertion.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
- Use min() function to cap width at viewport - 32px - Add maxWidth constraint for better mobile support - Update package-lock.json with new dependencies 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
server/index.js (1)
1097-1171: Fix broken token-usage endpoint: code sits outside a route and uses sync FSThis block is not inside an Express handler, causing syntax/parse errors and a 100% broken server (Illegal return outside function). Also uses fs.existsSync/readFileSync on the request path. Wrap it in a proper route and switch to async fs.promises.
Apply:
-// … previous code unchanged … - // Construct the JSONL file path - // Claude stores session files in ~/.claude/projects/[encoded-project-path]/[session-id].jsonl - // The encoding replaces /, spaces, ~, and _ with - - const homeDir = os.homedir(); - const encodedPath = projectPath.replace(/[\\/:\s~_]/g, '-'); - const projectDir = path.join(homeDir, '.claude', 'projects', encodedPath); - // Allow only safe characters in sessionId - const safeSessionId = String(sessionId).replace(/[^a-zA-Z0-9._-]/g, ''); - if (!safeSessionId) { - return res.status(400).json({ error: 'Invalid sessionId' }); - } - const jsonlPath = path.join(projectDir, `${safeSessionId}.jsonl`); - // Constrain to projectDir - const rel = path.relative(path.resolve(projectDir), path.resolve(jsonlPath)); - if (rel.startsWith('..') || path.isAbsolute(rel)) { - return res.status(400).json({ error: 'Invalid path' }); - } - - // Check if file exists - if (!fs.existsSync(jsonlPath)) { - return res.status(404).json({ error: 'Session file not found', path: jsonlPath }); - } -// … following code unchanged … - } - - // Read and parse the JSONL file - const fileContent = fs.readFileSync(jsonlPath, 'utf8'); - const lines = fileContent.trim().split('\n'); - - const parsedContextWindow = parseInt(process.env.CONTEXT_WINDOW, 10); - const contextWindow = Number.isFinite(parsedContextWindow) ? parsedContextWindow : 160000; - let inputTokens = 0; - let cacheCreationTokens = 0; - let cacheReadTokens = 0; - - // Find the latest assistant message with usage data (scan from end) - for (let i = lines.length - 1; i >= 0; i--) { - try { - const entry = JSON.parse(lines[i]); - - // Only count assistant messages which have usage data - if (entry.type === 'assistant' && entry.message?.usage) { - const usage = entry.message.usage; - - // Use token counts from latest assistant message only - inputTokens = usage.input_tokens || 0; - cacheCreationTokens = usage.cache_creation_input_tokens || 0; - cacheReadTokens = usage.cache_read_input_tokens || 0; - - break; // Stop after finding the latest assistant message - } - } catch (parseError) { - // Skip lines that can't be parsed - continue; - } - } - - // Calculate total context usage (excluding output_tokens, as per ccusage) - const totalUsed = inputTokens + cacheCreationTokens + cacheReadTokens; - - res.json({ - used: totalUsed, - total: contextWindow, - breakdown: { - input: inputTokens, - cacheCreation: cacheCreationTokens, - cacheRead: cacheReadTokens - } - }); - } catch (error) { - console.error('Error reading session token usage:', error); - res.status(500).json({ error: 'Failed to read session token usage' }); - } -}); +// Token usage endpoint +app.get('/api/sessions/:sessionId/token-usage', authenticateToken, async (req, res) => { + try { + const { sessionId } = req.params; + const { projectPath } = req.query; + if (!projectPath) { + return res.status(400).json({ error: 'projectPath query parameter is required' }); + } + // Validate sessionId + if (!/^[A-Za-z0-9._-]+$/.test(sessionId)) { + return res.status(400).json({ error: 'Invalid sessionId' }); + } + + // Construct the JSONL file path + const homeDir = os.homedir(); + const encodedPath = String(projectPath).replace(/[\\/:\s~_]/g, '-'); + const projectDir = path.join(homeDir, '.claude', 'projects', encodedPath); + const safeSessionId = String(sessionId).replace(/[^a-zA-Z0-9._-]/g, ''); + if (!safeSessionId) { + return res.status(400).json({ error: 'Invalid sessionId' }); + } + const jsonlPath = path.join(projectDir, `${safeSessionId}.jsonl`); + const rel = path.relative(path.resolve(projectDir), path.resolve(jsonlPath)); + if (rel.startsWith('..') || path.isAbsolute(rel)) { + return res.status(400).json({ error: 'Invalid path' }); + } + + // Check if file exists (async) + try { + await fsPromises.access(jsonlPath); + } catch { + return res.status(404).json({ error: 'Session file not found', path: jsonlPath }); + } + + // Read and parse the JSONL file (async) + const fileContent = await fsPromises.readFile(jsonlPath, 'utf8'); + const trimmed = fileContent.trim(); + const lines = trimmed ? trimmed.split('\n') : []; + + const parsedContextWindow = parseInt(process.env.CONTEXT_WINDOW, 10); + const contextWindow = Number.isFinite(parsedContextWindow) ? parsedContextWindow : 160000; + let inputTokens = 0; + let cacheCreationTokens = 0; + let cacheReadTokens = 0; + + // Find the latest assistant message with usage data (scan from end) + for (let i = lines.length - 1; i >= 0; i--) { + try { + const entry = JSON.parse(lines[i]); + if (entry.type === 'assistant' && entry.message?.usage) { + const usage = entry.message.usage; + inputTokens = usage.input_tokens || 0; + cacheCreationTokens = usage.cache_creation_input_tokens || 0; + cacheReadTokens = usage.cache_read_input_tokens || 0; + break; + } + } catch { + // ignore parse errors + } + } + + // Calculate total context usage (exclude output tokens) + const used = inputTokens + cacheCreationTokens + cacheReadTokens; + const percentage = contextWindow > 0 ? Math.round((used / contextWindow) * 100) : 0; + + return res.json({ + used, + total: contextWindow, + percentage, + breakdown: { + input: inputTokens, + cacheCreation: cacheCreationTokens, + cacheRead: cacheReadTokens + } + }); + } catch (error) { + console.error('Error reading session token usage:', error); + return res.status(500).json({ error: 'Failed to read session token usage' }); + } +});This resolves the parse errors and removes blocking sync FS on the request path. As per static analysis hints
Also applies to: 1123-1166
🧹 Nitpick comments (4)
src/components/ChatInterface.jsx (4)
2387-2569: Wrap switch-case bodies that declare consts (noSwitchDeclarations)Several cases declare consts without a surrounding block, tripping Biome and risking TDZ/hoisting surprises. Wrap them in braces.
Apply:
- case 'claude-response': - const messageData = latestMessage.data.message || latestMessage.data; + case 'claude-response': { + const messageData = latestMessage.data.message || latestMessage.data; ... - break; + break; + } @@ - case 'cursor-result': + case 'cursor-result': { // Get session ID from message or fall back to current session const cursorCompletedSessionId = latestMessage.sessionId || currentSessionId; ... - break; + break; + } @@ - case 'claude-complete': + case 'claude-complete': { // Get session ID from message or fall back to current session const completedSessionId = latestMessage.sessionId || currentSessionId || sessionStorage.getItem('pendingSessionId'); ... - break; + break; + } @@ - case 'session-aborted': + case 'session-aborted': { // Get session ID from message or fall back to current session const abortedSessionId = latestMessage.sessionId || currentSessionId; ... - break; + break; + }This clears the Biome errors. As per static analysis hints
Also applies to: 2672-2735, 2769-2805, 2807-2833
1428-1492: Avoid race between selection UI and command execution; robust arg parsingSelecting a command executes immediately via handleCommandSelect before updating input, so args parsed from input are often empty. Also, RegExp uses command.name without escaping.
Recommend:
- Either don’t auto-execute on selection; just insert the command and let the user add args.
- Or pass args explicitly and escape the command name when parsing.
Minimal code change to make execution robust:
-const executeCommand = useCallback(async (command) => { +const escapeRegExp = (s) => s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +const executeCommand = useCallback(async (command, argsOverride) => { if (!command || !selectedProject) return; try { - const commandMatch = input.match(new RegExp(`${command.name}\\s*(.*)`)); - const args = commandMatch && commandMatch[1] - ? commandMatch[1].trim().split(/\s+/) - : []; + const argsFromInput = (() => { + const re = new RegExp(`${escapeRegExp(command.name)}\\s*(.*)`); + const m = input.match(re); + return m && m[1] ? m[1].trim().split(/\s+/) : []; + })(); + const args = Array.isArray(argsOverride) ? argsOverride : argsFromInput; @@ - // Clear the input after successful execution - setInput(''); + // Clear input after successful execution + setInput('');And trigger execution after updating the input in selectCommand (or don’t execute here at all):
- // Track command usage - handleCommandSelect(command); + // Track command usage (history only) + handleCommandSelect(command); const newInput = textBeforeSlash + command.name + ' ' + textAfterQuery; const newCursorPos = textBeforeSlash.length + command.name.length + 1; // Update input and cursor position setInput(newInput); setCursorPosition(newCursorPos); @@ - // Hide command menu + // Hide command menu setShowCommandMenu(false); @@ - // Set cursor position + // Set cursor position if (textareaRef.current) { requestAnimationFrame(() => { if (textareaRef.current) { textareaRef.current.setSelectionRange(newCursorPos, newCursorPos); if (!textareaRef.current.matches(':focus')) { textareaRef.current.focus(); } } }); } + // Optionally auto-execute with no args (adjust as needed for metadata) + executeCommand(command, []);Alternatively, remove auto-execution here and let Enter send the command.
Also applies to: 1399-1426, 3396-3434
4061-4083: Clamp command menu position to viewportUsing top = textarea.top − 308 can push the menu off-screen on small viewports. Compute position with clamping:
- If top < 8, show below the textarea (textarea.bottom + 8).
- Ensure left doesn’t overflow (min 8, max window.innerWidth − menuWidth − 8).
This improves UX in varied screen sizes.
3053-3146: Reduce noisy console logs in token pollingThe polling effect logs on every interval. Gate under dev or remove to avoid console spam in prod.
Example:
- Wrap logs with if (import.meta.env.DEV) console.log(...)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
server/index.js(7 hunks)server/routes/commands.js(1 hunks)server/utils/commandParser.js(1 hunks)src/components/ChatInterface.jsx(30 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- server/utils/commandParser.js
- server/routes/commands.js
🧰 Additional context used
🧬 Code graph analysis (2)
server/index.js (4)
server/routes/commands.js (1)
sessionId(221-221)server/claude-sdk.js (6)
activeSessions(21-21)contextWindow(170-170)inputTokens(163-163)cacheCreationTokens(166-166)cacheReadTokens(165-165)totalUsed(169-169)server/projects.js (6)
projectDir(271-271)projectDir(525-525)projectDir(714-714)projectDir(801-801)projectDir(864-864)projectDir(904-904)server/cursor-cli.js (1)
process(239-239)
src/components/ChatInterface.jsx (3)
src/App.jsx (2)
showThinking(60-60)processingSessions(71-71)src/utils/api.js (3)
authenticatedFetch(2-20)authenticatedFetch(2-20)url(53-53)src/components/CommandMenu.jsx (1)
CommandMenu(13-290)
🪛 Biome (2.1.2)
server/index.js
[error] 1109-1110: Illegal return statement outside of a function
(parse)
[error] 1117-1118: Illegal return statement outside of a function
(parse)
[error] 1120-1124: Illegal return statement outside of a function
(parse)
[error] 1124-1124: Expected a statement but instead found '}'.
Expected a statement here.
(parse)
[error] 1169-1169: Expected a statement but instead found '} catch (error)'.
Expected a statement here.
(parse)
[error] 1174-1174: Expected a statement but instead found '})'.
Expected a statement here.
(parse)
src/components/ChatInterface.jsx
[error] 2388-2390: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2673-2675: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2725-2726: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2770-2772: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2809-2811: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (5)
server/index.js (3)
196-198: Commands API route registration looks goodProtected under authenticateToken and mounted at /api/commands.
558-560: Claude SDK integration and session status plumbing LGTMRouting claude-command via SDK, abort flows, and active-session queries are clean and consistent.
Also applies to: 577-585, 601-630
1275-1283: Good: Explicitly logging SDK mode and starting projects watcherClear operational signal at startup; watcher awaited within listen callback.
src/components/ChatInterface.jsx (2)
2523-2539: Nice: Normalize HTML entities and usage-limit time in assistant textGood user-facing polish.
1069-1081: Reasoning accordion gated by showThinking looks goodOpt-in display avoids clutter.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/ChatInterface.jsx (2)
2672-2735: Wrap ‘cursor-result’ case body in a block (fix noSwitchDeclarations).Scope consts and prevent fall‑through issues.
- case 'cursor-result': - // Get session ID from message or fall back to current session - const cursorCompletedSessionId = latestMessage.sessionId || currentSessionId; + case 'cursor-result': { + const cursorCompletedSessionId = latestMessage.sessionId || currentSessionId; @@ - break; + break; + }
2769-2805: Wrap ‘claude-complete’ case body in a block.- case 'claude-complete': + case 'claude-complete': { @@ - break; + break; + }
♻️ Duplicate comments (3)
server/index.js (1)
1097-1172: Fix malformed token‑usage endpoint and remove sync fs (parse + perf).The block is outside any handler (Biome “Illegal return…”). Also uses fs.existsSync/readFileSync on request path. Wrap in an Express route and use async fsPromises.
Apply:
-// … previous code unchanged … - // Construct the JSONL file path - // Claude stores session files in ~/.claude/projects/[encoded-project-path]/[session-id].jsonl - // The encoding replaces /, spaces, ~, and _ with - - const homeDir = os.homedir(); - const encodedPath = projectPath.replace(/[\\/:\s~_]/g, '-'); - const projectDir = path.join(homeDir, '.claude', 'projects', encodedPath); - // Allow only safe characters in sessionId - const safeSessionId = String(sessionId).replace(/[^a-zA-Z0-9._-]/g, ''); - if (!safeSessionId) { - return res.status(400).json({ error: 'Invalid sessionId' }); - } - const jsonlPath = path.join(projectDir, `${safeSessionId}.jsonl`); - // Constrain to projectDir - const rel = path.relative(path.resolve(projectDir), path.resolve(jsonlPath)); - if (rel.startsWith('..') || path.isAbsolute(rel)) { - return res.status(400).json({ error: 'Invalid path' }); - } - - // Check if file exists - if (!fs.existsSync(jsonlPath)) { - return res.status(404).json({ error: 'Session file not found', path: jsonlPath }); - } -// … following code unchanged … - } - - // Read and parse the JSONL file - const fileContent = fs.readFileSync(jsonlPath, 'utf8'); - const lines = fileContent.trim().split('\n'); - - const parsedContextWindow = parseInt(process.env.CONTEXT_WINDOW, 10); - const contextWindow = Number.isFinite(parsedContextWindow) ? parsedContextWindow : 160000; - let inputTokens = 0; - let cacheCreationTokens = 0; - let cacheReadTokens = 0; - - // Find the latest assistant message with usage data (scan from end) - for (let i = lines.length - 1; i >= 0; i--) { - try { - const entry = JSON.parse(lines[i]); - - // Only count assistant messages which have usage data - if (entry.type === 'assistant' && entry.message?.usage) { - const usage = entry.message.usage; - - // Use token counts from latest assistant message only - inputTokens = usage.input_tokens || 0; - cacheCreationTokens = usage.cache_creation_input_tokens || 0; - cacheReadTokens = usage.cache_read_input_tokens || 0; - - break; // Stop after finding the latest assistant message - } - } catch (parseError) { - // Skip lines that can't be parsed - continue; - } - } - - // Calculate total context usage (excluding output_tokens, as per ccusage) - const totalUsed = inputTokens + cacheCreationTokens + cacheReadTokens; - - res.json({ - used: totalUsed, - total: contextWindow, - breakdown: { - input: inputTokens, - cacheCreation: cacheCreationTokens, - cacheRead: cacheReadTokens - } - }); - } catch (error) { - console.error('Error reading session token usage:', error); - res.status(500).json({ error: 'Failed to read session token usage' }); - } -}); +// Per-session token usage (JSONL aggregate) +app.get('/api/sessions/:sessionId/token-usage', authenticateToken, async (req, res) => { + try { + const { sessionId } = req.params; + const { projectPath } = req.query; + if (!projectPath) { + return res.status(400).json({ error: 'projectPath query parameter is required' }); + } + // Validate sessionId to avoid traversal + if (!/^[A-Za-z0-9._-]+$/.test(String(sessionId))) { + return res.status(400).json({ error: 'Invalid sessionId' }); + } + + // ~/.claude/projects/[encoded-project-path]/[session-id].jsonl + const homeDir = os.homedir(); + const encodedPath = String(projectPath).replace(/[\\/:\s~_]/g, '-'); + const projectDir = path.join(homeDir, '.claude', 'projects', encodedPath); + const safeSessionId = String(sessionId).replace(/[^A-Za-z0-9._-]/g, ''); + const jsonlPath = path.join(projectDir, `${safeSessionId}.jsonl`); + const rel = path.relative(path.resolve(projectDir), path.resolve(jsonlPath)); + if (rel.startsWith('..') || path.isAbsolute(rel)) { + return res.status(400).json({ error: 'Invalid path' }); + } + + // Async existence/read + try { + await fsPromises.access(jsonlPath); + } catch { + return res.status(404).json({ error: 'Session file not found', path: jsonlPath }); + } + const fileContent = await fsPromises.readFile(jsonlPath, 'utf8'); + const lines = fileContent.trim().split('\n'); + + const parsedContextWindow = parseInt(process.env.CONTEXT_WINDOW, 10); + const contextWindow = Number.isFinite(parsedContextWindow) ? parsedContextWindow : 160000; + let inputTokens = 0; + let cacheCreationTokens = 0; + let cacheReadTokens = 0; + + // Scan from end for latest assistant usage + for (let i = lines.length - 1; i >= 0; i--) { + try { + const entry = JSON.parse(lines[i]); + if (entry.type === 'assistant' && entry.message?.usage) { + const usage = entry.message.usage; + inputTokens = usage.input_tokens || 0; + cacheCreationTokens = usage.cache_creation_input_tokens || 0; + cacheReadTokens = usage.cache_read_input_tokens || 0; + break; + } + } catch { + // ignore parse errors + } + } + + const totalUsed = inputTokens + cacheCreationTokens + cacheReadTokens; + return res.json({ + used: totalUsed, + total: contextWindow, + breakdown: { + input: inputTokens, + cacheCreation: cacheCreationTokens, + cacheRead: cacheReadTokens + } + }); + } catch (error) { + console.error('Error reading session token usage:', error); + return res.status(500).json({ error: 'Failed to read session token usage' }); + } +});Based on learnings
server/utils/commandParser.js (1)
4-8: Eliminate shell injection: stop using exec with a shell; enforce strict allowlist.Current flow allows “ls && rm …” because only the first token is checked and a shell interprets metacharacters.
Apply:
-import { exec } from 'child_process'; +import { execFile } from 'child_process'; import { promisify } from 'util'; -const execAsync = promisify(exec); +const execFileAsync = promisify(execFile);export function isBashCommandAllowed(command) { const trimmedCommand = command.trim(); if (!trimmedCommand) return false; // Extract the first word (the actual command) const firstWord = trimmedCommand.split(/\s+/)[0]; // Check if it's in the allowlist - return BASH_COMMAND_ALLOWLIST.some(allowed => - firstWord === allowed || firstWord.startsWith(allowed + '/') - ); + return BASH_COMMAND_ALLOWLIST.includes(firstWord); }- const { stdout, stderr } = await execAsync(command, { - cwd, - timeout, - maxBuffer: 1024 * 1024, // 1MB max output - shell: '/bin/bash' - }); + // Reject shell metacharacters outright + if (/[;&|`$<>]/.test(command)) { + throw new Error('Command contains disallowed shell characters'); + } + // Simple split; for full quoting support, switch to a shell-words parser + const [cmd, ...args] = command.split(/\s+/); + const { stdout, stderr } = await execFileAsync(cmd, args, { + cwd, + timeout, + maxBuffer: 1024 * 1024 + });Based on learnings
Also applies to: 149-160, 213-218
src/components/ChatInterface.jsx (1)
2809-2834: Wrap ‘session-aborted’ case body in a block.- case 'session-aborted': + case 'session-aborted': { @@ - break; + break; + }
🧹 Nitpick comments (3)
src/components/CommandMenu.jsx (2)
70-77: Normalize namespace to ensure correct grouping.If server omits namespace and only provides type ('built-in'/'custom'), items won’t appear under expected groups.
Apply:
- const groupedCommands = commands.reduce((groups, command) => { - const namespace = command.namespace || command.type || 'other'; + const groupedCommands = commands.reduce((groups, command) => { + const mapped = + command.namespace + ?? (command.type === 'built-in' ? 'builtin' + : command.type === 'custom' ? 'project' + : command.type || 'other'); + const namespace = mapped; if (!groups[namespace]) { groups[namespace] = []; }Also applies to: 84-89
152-159: Avoid O(n²) lookup when resolving globalIndex.Precompute a Map from (namespace,name) to index instead of Array.find.
src/components/ChatInterface.jsx (1)
4073-4081: Clamp CommandMenu position to the viewport (optional).Avoid off‑screen placement by clamping coordinates.
- position={{ - top: textareaRef.current - ? textareaRef.current.getBoundingClientRect().top - 308 - : 0, - left: textareaRef.current - ? textareaRef.current.getBoundingClientRect().left - : 0 - }} + position={(() => { + if (!textareaRef.current) return { top: 0, left: 0 }; + const rect = textareaRef.current.getBoundingClientRect(); + const vw = window.innerWidth || document.documentElement.clientWidth; + const vh = window.innerHeight || document.documentElement.clientHeight; + const menuH = 300 + 8; // height + padding + const menuW = Math.min(400, vw - 32); + const top = Math.max(8, Math.min(rect.top - menuH, vh - menuH - 8)); + const left = Math.max(8, Math.min(rect.left, vw - menuW - 16)); + return { top, left }; + })()}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
server/index.js(7 hunks)server/routes/commands.js(1 hunks)server/utils/commandParser.js(1 hunks)src/components/ChatInterface.jsx(30 hunks)src/components/CommandMenu.jsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- server/routes/commands.js
🧰 Additional context used
🧬 Code graph analysis (4)
server/index.js (4)
server/routes/commands.js (3)
sessionId(221-221)homeDir(394-394)__dirname(9-9)server/claude-sdk.js (6)
activeSessions(21-21)contextWindow(170-170)inputTokens(163-163)cacheCreationTokens(166-166)cacheReadTokens(165-165)totalUsed(169-169)server/projects.js (6)
projectDir(271-271)projectDir(525-525)projectDir(714-714)projectDir(801-801)projectDir(864-864)projectDir(904-904)server/cursor-cli.js (1)
process(239-239)
src/components/ChatInterface.jsx (4)
src/App.jsx (5)
showThinking(60-60)selectedProject(49-49)selectedSession(50-50)processingSessions(71-71)sendByCtrlEnter(62-62)src/utils/api.js (3)
authenticatedFetch(2-20)authenticatedFetch(2-20)url(53-53)server/routes/commands.js (5)
allCommands(380-380)result(492-492)content(39-39)content(448-448)content(533-533)src/components/CommandMenu.jsx (1)
CommandMenu(13-292)
server/utils/commandParser.js (4)
server/routes/mcp.js (1)
exec(22-22)server/index.js (8)
content(384-384)result(227-227)result(244-244)i(1134-1134)filePath(402-402)path(1016-1016)fileContent(1124-1124)fs(1017-1017)server/routes/commands.js (7)
content(39-39)content(448-448)content(533-533)result(492-492)resolvedPath(438-438)resolvedPath(517-517)processedContent(536-536)src/components/ChatInterface.jsx (2)
parsed(97-97)parsed(139-139)
src/components/CommandMenu.jsx (1)
src/components/ChatInterface.jsx (1)
frequentCommands(1371-1397)
🪛 Biome (2.1.2)
server/index.js
[error] 1109-1110: Illegal return statement outside of a function
(parse)
[error] 1117-1118: Illegal return statement outside of a function
(parse)
[error] 1120-1124: Illegal return statement outside of a function
(parse)
[error] 1124-1124: Expected a statement but instead found '}'.
Expected a statement here.
(parse)
[error] 1169-1169: Expected a statement but instead found '} catch (error)'.
Expected a statement here.
(parse)
[error] 1174-1174: Expected a statement but instead found '})'.
Expected a statement here.
(parse)
src/components/ChatInterface.jsx
[error] 2388-2390: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2673-2675: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2725-2726: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2770-2772: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2809-2811: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (4)
server/index.js (4)
196-198: Commands API route wiring looks good.Protected mount under /api/commands aligns with the new commands system.
558-560: WS: session control handlers (SDK + status) are solid.Provider‑aware abort, isActive checks, and active‑sessions listing look correct.
Also applies to: 575-585, 601-630
1181-1182: Dev redirect LGTM.Vite port default fallback works.
1275-1283: Good: explicit SDK mode log and awaiting watcher.Awaiting setupProjectsWatcher avoids races on startup.
…le operations Security Fixes: - Replace blocking fs.existsSync/readFileSync with async fs.promises.readFile in token usage endpoint - Implement comprehensive command injection protection using shell-quote parser - Validate commands against exact allowlist matches (no dangerous prefix matching) - Detect and block shell operators (&&, ||, |, ;, etc.) and metacharacters - Execute commands with execFile (shell: false) to prevent shell interpretation - Add argument validation to reject dangerous characters Bug Fixes: - Remove premature handleCommandSelect call from selectCommand to prevent double-counting usage - Add block scoping to 'session-aborted' switch case to prevent variable conflicts - Fix case fall-through by properly scoping const declarations with braces Technical Details: - server/index.js: Replace sync file ops with await fsPromises.readFile() - server/utils/commandParser.js: Complete security overhaul with shell-quote integration - src/components/ChatInterface.jsx: Command selection now only inserts text, execution happens on send 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Fixed syntax error caused by orphaned code at lines 1097-1114 - Added proper app.get endpoint definition for token-usage API - Wrapped code in async (req, res) handler with authentication middleware - Preserves all security features (async file reads, path validation) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Constrain file reads to project root directory - Constrain binary file serving to project root - Constrain file writes to project root - Use extractProjectDirectory to get actual project path - Validate resolved paths start with normalized project root - Prevent authenticated users from accessing files outside their projects Fixes path traversal vulnerability in: - GET /api/projects/:projectName/file (read endpoint) - GET /api/projects/:projectName/files/content (binary serve endpoint) - PUT /api/projects/:projectName/file (save endpoint) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/components/ChatInterface.jsx (1)
4060-4081: Do not execute on selection; only insert the command (and unify usage tracking).Click selection currently executes immediately via handleCommandSelect, while keyboard inserts. Make both paths insert-only; trigger execution explicitly on submit.
- <CommandMenu + <CommandMenu commands={filteredCommands} selectedIndex={selectedCommandIndex} - onSelect={handleCommandSelect} + onSelect={(cmd, idx, isHover) => { + if (isHover) { setSelectedCommandIndex(idx); return; } + selectCommand(cmd); + }}Consider moving “command usage history” increments to the execution point (e.g., when sending), so mouse and keyboard paths are consistent.
🧹 Nitpick comments (5)
server/utils/commandParser.js (2)
197-208: Escape bracket characters in dangerousPattern.Make the intent explicit and avoid char-class pitfalls with ‘]’.
- const dangerousPattern = /[;&|`$()<>{}[\]\\]/; + const dangerousPattern = /[;&|`$()<>{}\[\]\\]/;
106-114: Preserve leading whitespace/newlines when resolving @includes.Current replace uses the full match and only preserves a single space. Capture the prefix and re-insert it to keep formatting intact.
- const includePattern = /(?:^|\s)@([^\s]+)/gm; + const includePattern = /(^|\s)@([^\s]+)/gm; @@ - for (const match of matches) { - const fullMatch = match[0]; - const filename = match[1]; + for (const match of matches) { + const fullMatch = match[0]; + const prefix = match[1] || ''; + const filename = match[2]; @@ - // Replace the @filename with the file content - result = result.replace(fullMatch, fullMatch.startsWith(' ') ? ' ' + processedContent : processedContent); + // Replace keeping the exact prefix + result = result.replace(fullMatch, `${prefix}${processedContent}`);Also applies to: 116-134
src/components/ChatInterface.jsx (3)
2387-2570: Wrap ‘claude-response’ case in a block (noSwitchDeclarations).Prevents const leakage across cases.
- case 'claude-response': + case 'claude-response': { const messageData = latestMessage.data.message || latestMessage.data; @@ - break; + break; + }
2671-2735: Wrap ‘cursor-result’ case in a block (noSwitchDeclarations).Avoid cross-case scope bleed for consts.
- case 'cursor-result': + case 'cursor-result': { @@ - break; + break; + }
2768-2806: Wrap ‘claude-complete’ case in a block (noSwitchDeclarations).Same scoping fix as above.
- case 'claude-complete': + case 'claude-complete': { @@ - break; + break; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
server/index.js(7 hunks)server/utils/commandParser.js(1 hunks)src/components/ChatInterface.jsx(30 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
server/utils/commandParser.js (2)
server/index.js (7)
content(384-384)result(227-227)result(244-244)i(1135-1135)filePath(402-402)path(1016-1016)fs(1017-1017)server/routes/commands.js (8)
content(39-39)content(448-448)content(533-533)result(492-492)resolvedPath(438-438)resolvedPath(517-517)processedContent(536-536)commandName(45-45)
server/index.js (4)
server/routes/commands.js (3)
sessionId(221-221)homeDir(394-394)__dirname(9-9)server/claude-sdk.js (6)
activeSessions(21-21)contextWindow(170-170)inputTokens(163-163)cacheCreationTokens(166-166)cacheReadTokens(165-165)totalUsed(169-169)server/projects.js (10)
projectDir(271-271)projectDir(525-525)projectDir(714-714)projectDir(801-801)projectDir(864-864)projectDir(904-904)lines(815-815)entry(300-300)entry(663-663)entry(738-738)server/cursor-cli.js (1)
process(239-239)
src/components/ChatInterface.jsx (4)
src/App.jsx (5)
showThinking(60-60)selectedProject(49-49)selectedSession(50-50)processingSessions(71-71)sendByCtrlEnter(62-62)src/utils/api.js (3)
authenticatedFetch(2-20)authenticatedFetch(2-20)url(53-53)server/routes/commands.js (5)
allCommands(380-380)result(492-492)content(39-39)content(448-448)content(533-533)src/components/CommandMenu.jsx (1)
CommandMenu(13-292)
🪛 Biome (2.1.2)
server/index.js
[error] 1109-1110: Illegal return statement outside of a function
(parse)
[error] 1116-1119: Illegal return statement outside of a function
(parse)
[error] 1124-1126: Illegal return statement outside of a function
(parse)
[error] 1170-1170: Expected a statement but instead found '} catch (error)'.
Expected a statement here.
(parse)
[error] 1175-1175: Expected a statement but instead found '})'.
Expected a statement here.
(parse)
src/components/ChatInterface.jsx
[error] 2388-2390: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2673-2675: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2725-2726: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2770-2772: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (4)
server/index.js (3)
196-198: Commands API wiring looks good.Route is protected and mounted under /api/commands.
558-560: LGTM: switched chat path to Claude Agents SDK.Clean delegation to queryClaudeSDK.
601-630: Session status/active-sessions WS handlers: solid.Provider split and helpers usage look correct.
server/utils/commandParser.js (1)
276-288: Good hardening: exec without shell and bounded resources.Using execFile, no shell, allowlist + timeout + maxBuffer.
server/index.js
Outdated
| // … previous code unchanged … | ||
| // Construct the JSONL file path | ||
| // Claude stores session files in ~/.claude/projects/[encoded-project-path]/[session-id].jsonl | ||
| // The encoding replaces /, spaces, ~, and _ with - | ||
| const homeDir = os.homedir(); | ||
| const encodedPath = projectPath.replace(/[\\/:\s~_]/g, '-'); | ||
| const projectDir = path.join(homeDir, '.claude', 'projects', encodedPath); | ||
| // Allow only safe characters in sessionId | ||
| const safeSessionId = String(sessionId).replace(/[^a-zA-Z0-9._-]/g, ''); | ||
| if (!safeSessionId) { | ||
| return res.status(400).json({ error: 'Invalid sessionId' }); | ||
| } | ||
| const jsonlPath = path.join(projectDir, `${safeSessionId}.jsonl`); | ||
| // Constrain to projectDir | ||
| const rel = path.relative(path.resolve(projectDir), path.resolve(jsonlPath)); | ||
| if (rel.startsWith('..') || path.isAbsolute(rel)) { | ||
| return res.status(400).json({ error: 'Invalid path' }); | ||
| } | ||
|
|
||
| // Read and parse the JSONL file | ||
| let fileContent; | ||
| try { | ||
| fileContent = await fsPromises.readFile(jsonlPath, 'utf8'); | ||
| } catch (error) { | ||
| if (error.code === 'ENOENT') { | ||
| return res.status(404).json({ error: 'Session file not found', path: jsonlPath }); | ||
| } | ||
| throw error; // Re-throw other errors to be caught by outer try-catch | ||
| } | ||
| const lines = fileContent.trim().split('\n'); | ||
|
|
||
| const parsedContextWindow = parseInt(process.env.CONTEXT_WINDOW, 10); | ||
| const contextWindow = Number.isFinite(parsedContextWindow) ? parsedContextWindow : 160000; | ||
| let inputTokens = 0; | ||
| let cacheCreationTokens = 0; | ||
| let cacheReadTokens = 0; | ||
|
|
||
| // Find the latest assistant message with usage data (scan from end) | ||
| for (let i = lines.length - 1; i >= 0; i--) { | ||
| try { | ||
| const entry = JSON.parse(lines[i]); | ||
|
|
||
| // Only count assistant messages which have usage data | ||
| if (entry.type === 'assistant' && entry.message?.usage) { | ||
| const usage = entry.message.usage; | ||
|
|
||
| // Use token counts from latest assistant message only | ||
| inputTokens = usage.input_tokens || 0; | ||
| cacheCreationTokens = usage.cache_creation_input_tokens || 0; | ||
| cacheReadTokens = usage.cache_read_input_tokens || 0; | ||
|
|
||
| break; // Stop after finding the latest assistant message | ||
| } | ||
| } catch (parseError) { | ||
| // Skip lines that can't be parsed | ||
| continue; | ||
| } | ||
| } | ||
|
|
||
| // Calculate total context usage (excluding output_tokens, as per ccusage) | ||
| const totalUsed = inputTokens + cacheCreationTokens + cacheReadTokens; | ||
|
|
||
| res.json({ | ||
| used: totalUsed, | ||
| total: contextWindow, | ||
| breakdown: { | ||
| input: inputTokens, | ||
| cacheCreation: cacheCreationTokens, | ||
| cacheRead: cacheReadTokens | ||
| } | ||
| }); | ||
| } catch (error) { | ||
| console.error('Error reading session token usage:', error); | ||
| res.status(500).json({ error: 'Failed to read session token usage' }); | ||
| } | ||
| }); | ||
|
|
There was a problem hiding this comment.
Fix: token-usage code is outside a route handler (parse errors, illegal returns).
The block starting here executes at top-level; return res... outside a function triggers Biome parse errors and breaks the server. Wrap it in an Express route, declare params/query, and keep your sanitization/async I/O.
Apply:
+// Token usage endpoint
+app.get('/api/sessions/:sessionId/token-usage', authenticateToken, async (req, res) => {
+ try {
+ const { sessionId } = req.params;
+ const { projectPath } = req.query;
+ if (!projectPath) {
+ return res.status(400).json({ error: 'projectPath query parameter is required' });
+ }
-// … previous code unchanged …
- // Construct the JSONL file path
- // Claude stores session files in ~/.claude/projects/[encoded-project-path]/[session-id].jsonl
- // The encoding replaces /, spaces, ~, and _ with -
- const homeDir = os.homedir();
- const encodedPath = projectPath.replace(/[\\/:\s~_]/g, '-');
- const projectDir = path.join(homeDir, '.claude', 'projects', encodedPath);
- // Allow only safe characters in sessionId
- const safeSessionId = String(sessionId).replace(/[^a-zA-Z0-9._-]/g, '');
- if (!safeSessionId) {
- return res.status(400).json({ error: 'Invalid sessionId' });
- }
- const jsonlPath = path.join(projectDir, `${safeSessionId}.jsonl`);
- // Constrain to projectDir
- const rel = path.relative(path.resolve(projectDir), path.resolve(jsonlPath));
- if (rel.startsWith('..') || path.isAbsolute(rel)) {
- return res.status(400).json({ error: 'Invalid path' });
- }
+ // Construct the JSONL file path
+ // Claude stores session files in ~/.claude/projects/[encoded-project-path]/[session-id].jsonl
+ // The encoding replaces /, spaces, ~, and _ with -
+ const homeDir = os.homedir();
+ const encodedPath = projectPath.replace(/[\\/:\s~_]/g, '-');
+ const projectDir = path.join(homeDir, '.claude', 'projects', encodedPath);
+ // Allow only safe characters in sessionId
+ const safeSessionId = String(sessionId).replace(/[^a-zA-Z0-9._-]/g, '');
+ if (!safeSessionId) {
+ return res.status(400).json({ error: 'Invalid sessionId' });
+ }
+ const jsonlPath = path.join(projectDir, `${safeSessionId}.jsonl`);
+ // Constrain to projectDir
+ const rel = path.relative(path.resolve(projectDir), path.resolve(jsonlPath));
+ if (rel.startsWith('..') || path.isAbsolute(rel)) {
+ return res.status(400).json({ error: 'Invalid path' });
+ }
- // Read and parse the JSONL file
- let fileContent;
- try {
- fileContent = await fsPromises.readFile(jsonlPath, 'utf8');
- } catch (error) {
- if (error.code === 'ENOENT') {
- return res.status(404).json({ error: 'Session file not found', path: jsonlPath });
- }
- throw error; // Re-throw other errors to be caught by outer try-catch
- }
- const lines = fileContent.trim().split('\n');
+ // Read and parse the JSONL file
+ let fileContent;
+ try {
+ fileContent = await fsPromises.readFile(jsonlPath, 'utf8');
+ } catch (error) {
+ if (error.code === 'ENOENT') {
+ return res.status(404).json({ error: 'Session file not found', path: jsonlPath });
+ }
+ throw error; // Re-throw other errors to be caught by outer try-catch
+ }
+ const lines = fileContent.trim().split('\n');
- const parsedContextWindow = parseInt(process.env.CONTEXT_WINDOW, 10);
- const contextWindow = Number.isFinite(parsedContextWindow) ? parsedContextWindow : 160000;
- let inputTokens = 0;
- let cacheCreationTokens = 0;
- let cacheReadTokens = 0;
+ const parsedContextWindow = parseInt(process.env.CONTEXT_WINDOW, 10);
+ const contextWindow = Number.isFinite(parsedContextWindow) ? parsedContextWindow : 160000;
+ let inputTokens = 0;
+ let cacheCreationTokens = 0;
+ let cacheReadTokens = 0;
- // Find the latest assistant message with usage data (scan from end)
- for (let i = lines.length - 1; i >= 0; i--) {
- try {
- const entry = JSON.parse(lines[i]);
- // Only count assistant messages which have usage data
- if (entry.type === 'assistant' && entry.message?.usage) {
- const usage = entry.message.usage;
- // Use token counts from latest assistant message only
- inputTokens = usage.input_tokens || 0;
- cacheCreationTokens = usage.cache_creation_input_tokens || 0;
- cacheReadTokens = usage.cache_read_input_tokens || 0;
- break; // Stop after finding the latest assistant message
- }
- } catch (parseError) {
- // Skip lines that can't be parsed
- continue;
- }
- }
+ // Find the latest assistant message with usage data (scan from end)
+ for (let i = lines.length - 1; i >= 0; i--) {
+ try {
+ const entry = JSON.parse(lines[i]);
+ if (entry.type === 'assistant' && entry.message?.usage) {
+ const usage = entry.message.usage;
+ inputTokens = usage.input_tokens || 0;
+ cacheCreationTokens = usage.cache_creation_input_tokens || 0;
+ cacheReadTokens = usage.cache_read_input_tokens || 0;
+ break;
+ }
+ } catch {
+ continue;
+ }
+ }
- // Calculate total context usage (excluding output_tokens, as per ccusage)
- const totalUsed = inputTokens + cacheCreationTokens + cacheReadTokens;
+ // Calculate total context usage (excluding output_tokens, as per ccusage)
+ const totalUsed = inputTokens + cacheCreationTokens + cacheReadTokens;
- res.json({
- used: totalUsed,
- total: contextWindow,
- breakdown: {
- input: inputTokens,
- cacheCreation: cacheCreationTokens,
- cacheRead: cacheReadTokens
- }
- });
- } catch (error) {
- console.error('Error reading session token usage:', error);
- res.status(500).json({ error: 'Failed to read session token usage' });
- }
-});
+ res.json({
+ used: totalUsed,
+ total: contextWindow,
+ breakdown: {
+ input: inputTokens,
+ cacheCreation: cacheCreationTokens,
+ cacheRead: cacheReadTokens
+ }
+ });
+ } catch (error) {
+ console.error('Error reading session token usage:', error);
+ res.status(500).json({ error: 'Failed to read session token usage' });
+ }
+});This resolves the Biome errors and restores the endpoint.
📝 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.
| // … previous code unchanged … | |
| // Construct the JSONL file path | |
| // Claude stores session files in ~/.claude/projects/[encoded-project-path]/[session-id].jsonl | |
| // The encoding replaces /, spaces, ~, and _ with - | |
| const homeDir = os.homedir(); | |
| const encodedPath = projectPath.replace(/[\\/:\s~_]/g, '-'); | |
| const projectDir = path.join(homeDir, '.claude', 'projects', encodedPath); | |
| // Allow only safe characters in sessionId | |
| const safeSessionId = String(sessionId).replace(/[^a-zA-Z0-9._-]/g, ''); | |
| if (!safeSessionId) { | |
| return res.status(400).json({ error: 'Invalid sessionId' }); | |
| } | |
| const jsonlPath = path.join(projectDir, `${safeSessionId}.jsonl`); | |
| // Constrain to projectDir | |
| const rel = path.relative(path.resolve(projectDir), path.resolve(jsonlPath)); | |
| if (rel.startsWith('..') || path.isAbsolute(rel)) { | |
| return res.status(400).json({ error: 'Invalid path' }); | |
| } | |
| // Read and parse the JSONL file | |
| let fileContent; | |
| try { | |
| fileContent = await fsPromises.readFile(jsonlPath, 'utf8'); | |
| } catch (error) { | |
| if (error.code === 'ENOENT') { | |
| return res.status(404).json({ error: 'Session file not found', path: jsonlPath }); | |
| } | |
| throw error; // Re-throw other errors to be caught by outer try-catch | |
| } | |
| const lines = fileContent.trim().split('\n'); | |
| const parsedContextWindow = parseInt(process.env.CONTEXT_WINDOW, 10); | |
| const contextWindow = Number.isFinite(parsedContextWindow) ? parsedContextWindow : 160000; | |
| let inputTokens = 0; | |
| let cacheCreationTokens = 0; | |
| let cacheReadTokens = 0; | |
| // Find the latest assistant message with usage data (scan from end) | |
| for (let i = lines.length - 1; i >= 0; i--) { | |
| try { | |
| const entry = JSON.parse(lines[i]); | |
| // Only count assistant messages which have usage data | |
| if (entry.type === 'assistant' && entry.message?.usage) { | |
| const usage = entry.message.usage; | |
| // Use token counts from latest assistant message only | |
| inputTokens = usage.input_tokens || 0; | |
| cacheCreationTokens = usage.cache_creation_input_tokens || 0; | |
| cacheReadTokens = usage.cache_read_input_tokens || 0; | |
| break; // Stop after finding the latest assistant message | |
| } | |
| } catch (parseError) { | |
| // Skip lines that can't be parsed | |
| continue; | |
| } | |
| } | |
| // Calculate total context usage (excluding output_tokens, as per ccusage) | |
| const totalUsed = inputTokens + cacheCreationTokens + cacheReadTokens; | |
| res.json({ | |
| used: totalUsed, | |
| total: contextWindow, | |
| breakdown: { | |
| input: inputTokens, | |
| cacheCreation: cacheCreationTokens, | |
| cacheRead: cacheReadTokens | |
| } | |
| }); | |
| } catch (error) { | |
| console.error('Error reading session token usage:', error); | |
| res.status(500).json({ error: 'Failed to read session token usage' }); | |
| } | |
| }); | |
| // … previous code unchanged … | |
| // Token usage endpoint | |
| app.get('/api/sessions/:sessionId/token-usage', authenticateToken, async (req, res) => { | |
| try { | |
| const { sessionId } = req.params; | |
| const { projectPath } = req.query; | |
| if (!projectPath) { | |
| return res.status(400).json({ error: 'projectPath query parameter is required' }); | |
| } | |
| // Construct the JSONL file path | |
| // Claude stores session files in ~/.claude/projects/[encoded-project-path]/[session-id].jsonl | |
| // The encoding replaces /, spaces, ~, and _ with - | |
| const homeDir = os.homedir(); | |
| const encodedPath = projectPath.replace(/[\\/:\s~_]/g, '-'); | |
| const projectDir = path.join(homeDir, '.claude', 'projects', encodedPath); | |
| // Allow only safe characters in sessionId | |
| const safeSessionId = String(sessionId).replace(/[^a-zA-Z0-9._-]/g, ''); | |
| if (!safeSessionId) { | |
| return res.status(400).json({ error: 'Invalid sessionId' }); | |
| } | |
| const jsonlPath = path.join(projectDir, `${safeSessionId}.jsonl`); | |
| // Constrain to projectDir | |
| const rel = path.relative(path.resolve(projectDir), path.resolve(jsonlPath)); | |
| if (rel.startsWith('..') || path.isAbsolute(rel)) { | |
| return res.status(400).json({ error: 'Invalid path' }); | |
| } | |
| // Read and parse the JSONL file | |
| let fileContent; | |
| try { | |
| fileContent = await fsPromises.readFile(jsonlPath, 'utf8'); | |
| } catch (error) { | |
| if (error.code === 'ENOENT') { | |
| return res.status(404).json({ error: 'Session file not found', path: jsonlPath }); | |
| } | |
| throw error; // Re-throw other errors to be caught by outer try-catch | |
| } | |
| const lines = fileContent.trim().split('\n'); | |
| const parsedContextWindow = parseInt(process.env.CONTEXT_WINDOW, 10); | |
| const contextWindow = Number.isFinite(parsedContextWindow) ? parsedContextWindow : 160000; | |
| let inputTokens = 0; | |
| let cacheCreationTokens = 0; | |
| let cacheReadTokens = 0; | |
| // Find the latest assistant message with usage data (scan from end) | |
| for (let i = lines.length - 1; i >= 0; i--) { | |
| try { | |
| const entry = JSON.parse(lines[i]); | |
| if (entry.type === 'assistant' && entry.message?.usage) { | |
| const usage = entry.message.usage; | |
| inputTokens = usage.input_tokens || 0; | |
| cacheCreationTokens = usage.cache_creation_input_tokens || 0; | |
| cacheReadTokens = usage.cache_read_input_tokens || 0; | |
| break; | |
| } | |
| } catch { | |
| continue; | |
| } | |
| } | |
| // Calculate total context usage (excluding output_tokens, as per ccusage) | |
| const totalUsed = inputTokens + cacheCreationTokens + cacheReadTokens; | |
| res.json({ | |
| used: totalUsed, | |
| total: contextWindow, | |
| breakdown: { | |
| input: inputTokens, | |
| cacheCreation: cacheCreationTokens, | |
| cacheRead: cacheReadTokens | |
| } | |
| }); | |
| } catch (error) { | |
| console.error('Error reading session token usage:', error); | |
| res.status(500).json({ error: 'Failed to read session token usage' }); | |
| } | |
| }); | |
| // … following code unchanged … |
🧰 Tools
🪛 Biome (2.1.2)
[error] 1109-1110: Illegal return statement outside of a function
(parse)
[error] 1116-1119: Illegal return statement outside of a function
(parse)
[error] 1124-1126: Illegal return statement outside of a function
(parse)
[error] 1170-1170: Expected a statement but instead found '} catch (error)'.
Expected a statement here.
(parse)
- Import WebSocket from 'ws' library - Change all instances from client.OPEN/ws.OPEN to WebSocket.OPEN - Fixed 4 occurrences: lines 111, 784, 831, 868 - Ensures correct WebSocket state checking using library constant Addresses CodeRabbit security review feedback. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
src/components/ChatInterface.jsx (4)
36-45: Consider expanding HTML entity coverage or using DOMParser.The current implementation only handles 5 common entities (
<,>,",',&). If the backend can send other entities (e.g., ,', numeric entities), they won't be decoded.More robust alternatives:
Option 1 (Simple expansion):
function decodeHtmlEntities(text) { if (!text) return text; return text .replace(/</g, '<') .replace(/>/g, '>') .replace(/"/g, '"') .replace(/'/g, "'") + .replace(/'/g, "'") + .replace(/ /g, ' ') .replace(/&/g, '&'); }Option 2 (DOMParser - handles all entities):
function decodeHtmlEntities(text) { if (!text) return text; - return text - .replace(/</g, '<') - .replace(/>/g, '>') - .replace(/"/g, '"') - .replace(/'/g, "'") - .replace(/&/g, '&'); + const doc = new DOMParser().parseFromString(text, 'text/html'); + return doc.documentElement.textContent; }Only add this if you've confirmed the backend sends other entities.
1427-1492: Add loading state during command execution.The command execution doesn't show loading feedback. If the API call is slow, the UI appears frozen.
Apply this diff to add loading state:
const executeCommand = useCallback(async (command) => { if (!command || !selectedProject) return; + setIsLoading(true); try { // ... existing code ... // Clear the input after successful execution setInput(''); setShowCommandMenu(false); setSlashPosition(-1); setCommandQuery(''); setSelectedCommandIndex(-1); } catch (error) { console.error('Error executing command:', error); // Show error message to user setChatMessages(prev => [...prev, { role: 'assistant', content: `Error executing command: ${error.message}`, timestamp: Date.now() }]); + } finally { + setIsLoading(false); } }, [input, selectedProject, currentSessionId, provider, cursorModel, tokenBudget]);
2387-2446: Consider wrapping streaming logic in a block for consistency.While the
messageDataconstant (line 2388) is technically case-scoped, wrapping the streaming logic in a block would be consistent with other cases and satisfy the linter.Apply this diff:
case 'claude-response': + { const messageData = latestMessage.data.message || latestMessage.data; // Handle Cursor streaming format (content_block_delta / content_block_stop) if (messageData && typeof messageData === 'object' && messageData.type) { // ... streaming logic ... } // ... rest of claude-response handling ... break; + }This is optional as the current code is functionally correct, but it maintains consistency with the other switch cases that were fixed.
4060-4081: Avoid hardcoded offset for CommandMenu positioning.
Remove the-308magic number and compute the top offset based on the actual rendered menu height (e.g. usingmenuRef.current.clientHeight) or delegate placement toCommandMenuby passing it thetextareaRefso it can internally calculate its own position.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
server/index.js(7 hunks)server/utils/commandParser.js(1 hunks)src/components/ChatInterface.jsx(30 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
server/utils/commandParser.js (2)
server/index.js (7)
content(384-384)result(227-227)result(244-244)i(1150-1150)filePath(402-402)path(1016-1016)fs(1017-1017)server/routes/commands.js (8)
content(39-39)content(448-448)content(533-533)result(492-492)resolvedPath(438-438)resolvedPath(517-517)processedContent(536-536)commandName(45-45)
server/index.js (4)
server/routes/commands.js (2)
sessionId(221-221)homeDir(394-394)server/claude-sdk.js (6)
activeSessions(21-21)contextWindow(170-170)inputTokens(163-163)cacheCreationTokens(166-166)cacheReadTokens(165-165)totalUsed(169-169)server/projects.js (8)
projectName(900-900)projectDir(271-271)projectDir(525-525)projectDir(714-714)projectDir(801-801)projectDir(864-864)projectDir(904-904)lines(815-815)server/cursor-cli.js (1)
process(239-239)
src/components/ChatInterface.jsx (3)
src/App.jsx (3)
showThinking(60-60)processingSessions(71-71)sendByCtrlEnter(62-62)src/utils/api.js (3)
authenticatedFetch(2-20)authenticatedFetch(2-20)url(53-53)src/components/CommandMenu.jsx (1)
CommandMenu(13-292)
🪛 Biome (2.1.2)
src/components/ChatInterface.jsx
[error] 2388-2390: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2673-2675: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2725-2726: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2770-2772: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (18)
server/utils/commandParser.js (1)
1-303: LGTM! Security implementations are robust.All previously identified critical security issues have been properly addressed:
- Directory traversal prevention (lines 80-89): Uses
path.relativeto ensure resolved paths don't escape the base directory- Shell injection prevention (lines 145-211, 244-303): Uses
shell-quoteparser to detect operators, validates against allowlist, checks arguments for metacharacters, and executes withexecFile(no shell interpretation)- Control character sanitization (lines 229-242): Uses charCode filtering instead of regex to avoid linter issues
The command parser provides a secure, well-structured API for processing markdown-based commands with proper error handling and recursion limits.
server/index.js (4)
49-49: LGTM! Commands API properly integrated.The new Commands API route is correctly imported and registered with authentication middleware, following the established pattern for other protected routes in the application.
Also applies to: 196-198
41-42: LGTM! Claude SDK integration is clean.The migration from CLI-based integration to the Claude Agents SDK is well-implemented:
- Session management functions properly imported from dedicated modules
- Provider-specific logic correctly routes to Claude SDK or Cursor functions
- New WebSocket message types (
check-session-status,get-active-sessions) enable real-time session monitoring- Backward compatibility maintained for existing message types
Also applies to: 558-559, 577-584, 601-629
1097-1187: LGTM! Token usage endpoint is secure and well-implemented.All previously identified security issues have been properly resolved:
- sessionId sanitization (lines 1119-1122): Only allows alphanumeric, dots, dashes, and underscores
- Path traversal prevention (lines 1125-1129): Uses
path.relativeto constrain to project directory- Async I/O (lines 1134-1140): Uses
fsPromises.readFileinstead of blocking sync operations- Windows compatibility (line 1115): Sanitizes backslashes and colons in path encoding
The token calculation logic correctly uses only the latest assistant message's usage data (matching
ccusagebehavior), excluding output tokens from context window calculation.Verify that the context window default of 160,000 tokens (line 1144) is appropriate for the Claude models being used. Consider documenting this default in the
.env.examplefile or configuration documentation.
1191-1198: LGTM! Startup configuration properly updated.The startup flow correctly:
- Logs the Claude Agents SDK usage for debugging
- Initializes the projects watcher to enable real-time updates
- Updates the dev server redirect to use Vite's default port (5173)
Also applies to: 1291-1292, 1298-1298
src/components/ChatInterface.jsx (13)
1284-1337: LGTM with one observation.The command fetching logic is solid. The effect correctly depends on
selectedProjectand sets an empty array on error.Minor observation: The error handling logs to console but doesn't show a user-facing error. This is likely fine for a non-critical feature, but consider if users should see a notification when commands fail to load.
1339-1352: LGTM!The Fuse instance is properly memoized to avoid expensive re-creation. The search configuration (threshold 0.4, weights favoring name over description) provides a good user experience for fuzzy command matching.
1586-1617: Verify custom command auto-execution.The comment "Automatically send to Claude for processing" suggests the command should be sent to the backend, but I only see
setChatMessageswhich adds it to the UI.Is the auto-send implemented elsewhere, or is the user expected to manually send after the command is inserted? If it should auto-execute, the code is missing a call to
handleSubmit()orsendMessage(). If manual send is intended, clarify the comment:- // Automatically send to Claude for processing - // The command content will be included in the next API call + // The command content is inserted into chat history + // User must manually send to trigger Claude processing
2207-2245: LGTM! Token budget reset on session change is correct.The logic properly distinguishes between initial load and session change, resetting pagination and token budget only when needed. The session status check via WebSocket is appropriate.
Minor note: This sends a
check-session-statusmessage on every session change. If users rapidly switch sessions, this could generate WebSocket traffic. Consider debouncing if session-switching UX allows rapid clicks.
2342-2359: Processing state coordination looks good.The two-effect pattern correctly tracks and restores processing state when switching sessions.
Edge case to be aware of: If
isLoadingis set totrueby user action, thencurrentSessionIdupdates in the same render, the first effect will fire with the new session ID. This should be fine as it correctly marks the session as processing, but be aware this can happen.
2382-2385: LGTM! Migration from WebSocket to HTTP polling is documented.The change from WebSocket to HTTP polling for token budgets is clearly documented with the console log. This is a reasonable architectural change if WebSocket token updates were unreliable.
2807-2834: LGTM! Case declarations are properly scoped.The
session-abortedcase is correctly wrapped in a block, scoping theabortedSessionIdconstant to prevent cross-case access. This matches the stated fix from past review comments.Note to reviewer: The static analysis still flags this, which suggests the tool may be running on an older commit. The code as shown is correct.
2836-2849: LGTM! Case declarations are properly scoped.The
session-statuscase is correctly wrapped in a block, scoping all constants. The logic for restoring processing state when switching to an active session is correct.Note: Static analysis flags this but the code is correct. Tool may be on stale commit.
3053-3147: Token polling implementation is solid with proper race condition handling.The token usage polling correctly handles common edge cases:
- Aborts in-flight requests when session/project changes
- Verifies session/project hasn't changed before updating state (lines 3101-3104)
- Resets to zero for new sessions
- Refreshes on visibility change
Minor observation: The verbose console logging (lines 3055-3062, 3094-3111) is helpful for debugging but consider reducing verbosity or using a feature flag in production to avoid console noise.
Example:
const DEBUG_TOKEN_POLLING = false; // or import.meta.env.DEV if (DEBUG_TOKEN_POLLING) { console.log('🔍 Token usage polling effect triggered', { ... }); }
3396-3433: LGTM! Command insertion is correct.The function properly inserts the command name (which already includes the leading
/) into the input, updates cursor position, and focuses the textarea usingrequestAnimationFramefor smooth updates.Note: Past review comments mentioned a double-slash issue that has been resolved. The current code correctly uses
command.namedirectly without adding an extra/.
3435-3472: LGTM! Keyboard navigation follows standard patterns.The command menu keyboard handling is well-implemented with:
- Arrow keys for navigation (wrap-around at boundaries)
- Tab/Enter for selection
- Escape to close
- Proper
preventDefault()and early returns
3575-3639: Slash command detection is well-implemented.The logic correctly:
- Detects slash commands only at start or after whitespace
- Avoids showing the menu in code blocks (backtick counting)
- Debounces query updates to reduce Fuse.js overhead
- Clears state when no slash is detected
Minor edge case: The code block detection counts triple backticks (line 3597), which works for most cases but could fail if:
- User types an odd number of backticks without closing (e.g.,
\``` followed by text)- Backticks in inline code (single backticks) affect the count
This is likely acceptable for the UX trade-off, as perfect parsing would require more complex logic.
4173-4216: LGTM! Slash commands button provides good discoverability.The button implementation is solid:
- Clear icon indicating it opens a menu
- Badge showing command count (useful feedback)
- Toggles menu and resets query to show all commands
- Focuses textarea after interaction
This makes slash commands more discoverable than relying solely on typing
/.
| // Streaming throttle buffers | ||
| const streamBufferRef = useRef(''); | ||
| const streamTimerRef = useRef(null); | ||
| const commandQueryTimerRef = useRef(null); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify timer cleanup on unmount.
The commandQueryTimerRef is used for debouncing command queries, but I don't see cleanup in a useEffect cleanup function.
Run this script to check for cleanup:
If no cleanup is found, add a cleanup effect:
useEffect(() => {
return () => {
if (commandQueryTimerRef.current) {
clearTimeout(commandQueryTimerRef.current);
}
};
}, []);This prevents the timer from firing after component unmount, which could cause state updates on unmounted components.
🏁 Script executed:
#!/bin/bash
# Search for cleanup of commandQueryTimerRef in useEffect cleanup functions
rg -n "commandQueryTimerRef" src/components/ChatInterface.jsx -A 3 -B 3 | grep -E "return.*=>|cleanup"Length of output: 0
🏁 Script executed:
#!/bin/bash
rg -n "clearTimeout" src/components/ChatInterface.jsx
rg -n "commandQueryTimerRef.current" src/components/ChatInterface.jsx -C3Length of output: 1471
Add unmount cleanup for the debounce timer
Include a useEffect with an empty dependency array to clear any pending timeout on unmount:
useEffect(() => {
return () => {
if (commandQueryTimerRef.current) {
clearTimeout(commandQueryTimerRef.current);
}
};
}, []);🤖 Prompt for AI Agents
In src/components/ChatInterface.jsx around line 1219, the ref for
commandQueryTimerRef is created but there is no cleanup on component unmount;
add a useEffect with an empty dependency array that returns a cleanup function
which checks commandQueryTimerRef.current and calls clearTimeout on it if
present to prevent stray timers from firing after unmount.
| case 'cursor-result': | ||
| // Handle Cursor completion and final result text | ||
| setIsLoading(false); | ||
| setCanAbortSession(false); | ||
| setClaudeStatus(null); | ||
| try { | ||
| const r = latestMessage.data || {}; | ||
| const textResult = typeof r.result === 'string' ? r.result : ''; | ||
| // Flush buffered deltas before finalizing | ||
| if (streamTimerRef.current) { | ||
| clearTimeout(streamTimerRef.current); | ||
| streamTimerRef.current = null; | ||
| // Get session ID from message or fall back to current session | ||
| const cursorCompletedSessionId = latestMessage.sessionId || currentSessionId; | ||
|
|
||
| // Only update UI state if this is the current session | ||
| if (cursorCompletedSessionId === currentSessionId) { | ||
| setIsLoading(false); | ||
| setCanAbortSession(false); | ||
| setClaudeStatus(null); | ||
| } | ||
|
|
||
| // Always mark the completed session as inactive and not processing | ||
| if (cursorCompletedSessionId) { | ||
| if (onSessionInactive) { | ||
| onSessionInactive(cursorCompletedSessionId); | ||
| } | ||
| if (onSessionNotProcessing) { | ||
| onSessionNotProcessing(cursorCompletedSessionId); | ||
| } | ||
| const pendingChunk = streamBufferRef.current; | ||
| streamBufferRef.current = ''; | ||
|
|
||
| setChatMessages(prev => { | ||
| const updated = [...prev]; | ||
| // Try to consolidate into the last streaming assistant message | ||
| const last = updated[updated.length - 1]; | ||
| if (last && last.type === 'assistant' && !last.isToolUse && last.isStreaming) { | ||
| // Replace streaming content with the final content so deltas don't remain | ||
| const finalContent = textResult && textResult.trim() ? textResult : (last.content || '') + (pendingChunk || ''); | ||
| last.content = finalContent; | ||
| last.isStreaming = false; | ||
| } else if (textResult && textResult.trim()) { | ||
| updated.push({ type: r.is_error ? 'error' : 'assistant', content: textResult, timestamp: new Date(), isStreaming: false }); | ||
| } | ||
| return updated; | ||
| }); | ||
| } catch (e) { | ||
| console.warn('Error handling cursor-result message:', e); | ||
| } | ||
|
|
||
| // Mark session as inactive | ||
| const cursorSessionId = currentSessionId || sessionStorage.getItem('pendingSessionId'); | ||
| if (cursorSessionId && onSessionInactive) { | ||
| onSessionInactive(cursorSessionId); | ||
|
|
||
| // Only process result for current session | ||
| if (cursorCompletedSessionId === currentSessionId) { | ||
| try { | ||
| const r = latestMessage.data || {}; | ||
| const textResult = typeof r.result === 'string' ? r.result : ''; | ||
| // Flush buffered deltas before finalizing | ||
| if (streamTimerRef.current) { | ||
| clearTimeout(streamTimerRef.current); | ||
| streamTimerRef.current = null; | ||
| } | ||
| const pendingChunk = streamBufferRef.current; | ||
| streamBufferRef.current = ''; | ||
|
|
||
| setChatMessages(prev => { | ||
| const updated = [...prev]; | ||
| // Try to consolidate into the last streaming assistant message | ||
| const last = updated[updated.length - 1]; | ||
| if (last && last.type === 'assistant' && !last.isToolUse && last.isStreaming) { | ||
| // Replace streaming content with the final content so deltas don't remain | ||
| const finalContent = textResult && textResult.trim() ? textResult : (last.content || '') + (pendingChunk || ''); | ||
| last.content = finalContent; | ||
| last.isStreaming = false; | ||
| } else if (textResult && textResult.trim()) { | ||
| updated.push({ type: r.is_error ? 'error' : 'assistant', content: textResult, timestamp: new Date(), isStreaming: false }); | ||
| } | ||
| return updated; | ||
| }); | ||
| } catch (e) { | ||
| console.warn('Error handling cursor-result message:', e); | ||
| } | ||
| } | ||
|
|
||
| // Store session ID for future use and trigger refresh | ||
| if (cursorSessionId && !currentSessionId) { | ||
| setCurrentSessionId(cursorSessionId); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Wrap case declarations in a block (noSwitchDeclarations).
The cursor-result case declares constants that can be accessed by other cases, violating noSwitchDeclarations. This matches the static analysis warning.
Apply this diff to wrap declarations in a block:
- case 'cursor-result':
- // Get session ID from message or fall back to current session
- const cursorCompletedSessionId = latestMessage.sessionId || currentSessionId;
-
+ case 'cursor-result': {
+ const cursorCompletedSessionId = latestMessage.sessionId || currentSessionId;
+
// ... rest of the case logic ...
break;
+ }This scopes all variables to the case block, preventing cross-case access.
📝 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.
| case 'cursor-result': | |
| // Handle Cursor completion and final result text | |
| setIsLoading(false); | |
| setCanAbortSession(false); | |
| setClaudeStatus(null); | |
| try { | |
| const r = latestMessage.data || {}; | |
| const textResult = typeof r.result === 'string' ? r.result : ''; | |
| // Flush buffered deltas before finalizing | |
| if (streamTimerRef.current) { | |
| clearTimeout(streamTimerRef.current); | |
| streamTimerRef.current = null; | |
| // Get session ID from message or fall back to current session | |
| const cursorCompletedSessionId = latestMessage.sessionId || currentSessionId; | |
| // Only update UI state if this is the current session | |
| if (cursorCompletedSessionId === currentSessionId) { | |
| setIsLoading(false); | |
| setCanAbortSession(false); | |
| setClaudeStatus(null); | |
| } | |
| // Always mark the completed session as inactive and not processing | |
| if (cursorCompletedSessionId) { | |
| if (onSessionInactive) { | |
| onSessionInactive(cursorCompletedSessionId); | |
| } | |
| if (onSessionNotProcessing) { | |
| onSessionNotProcessing(cursorCompletedSessionId); | |
| } | |
| const pendingChunk = streamBufferRef.current; | |
| streamBufferRef.current = ''; | |
| setChatMessages(prev => { | |
| const updated = [...prev]; | |
| // Try to consolidate into the last streaming assistant message | |
| const last = updated[updated.length - 1]; | |
| if (last && last.type === 'assistant' && !last.isToolUse && last.isStreaming) { | |
| // Replace streaming content with the final content so deltas don't remain | |
| const finalContent = textResult && textResult.trim() ? textResult : (last.content || '') + (pendingChunk || ''); | |
| last.content = finalContent; | |
| last.isStreaming = false; | |
| } else if (textResult && textResult.trim()) { | |
| updated.push({ type: r.is_error ? 'error' : 'assistant', content: textResult, timestamp: new Date(), isStreaming: false }); | |
| } | |
| return updated; | |
| }); | |
| } catch (e) { | |
| console.warn('Error handling cursor-result message:', e); | |
| } | |
| // Mark session as inactive | |
| const cursorSessionId = currentSessionId || sessionStorage.getItem('pendingSessionId'); | |
| if (cursorSessionId && onSessionInactive) { | |
| onSessionInactive(cursorSessionId); | |
| // Only process result for current session | |
| if (cursorCompletedSessionId === currentSessionId) { | |
| try { | |
| const r = latestMessage.data || {}; | |
| const textResult = typeof r.result === 'string' ? r.result : ''; | |
| // Flush buffered deltas before finalizing | |
| if (streamTimerRef.current) { | |
| clearTimeout(streamTimerRef.current); | |
| streamTimerRef.current = null; | |
| } | |
| const pendingChunk = streamBufferRef.current; | |
| streamBufferRef.current = ''; | |
| setChatMessages(prev => { | |
| const updated = [...prev]; | |
| // Try to consolidate into the last streaming assistant message | |
| const last = updated[updated.length - 1]; | |
| if (last && last.type === 'assistant' && !last.isToolUse && last.isStreaming) { | |
| // Replace streaming content with the final content so deltas don't remain | |
| const finalContent = textResult && textResult.trim() ? textResult : (last.content || '') + (pendingChunk || ''); | |
| last.content = finalContent; | |
| last.isStreaming = false; | |
| } else if (textResult && textResult.trim()) { | |
| updated.push({ type: r.is_error ? 'error' : 'assistant', content: textResult, timestamp: new Date(), isStreaming: false }); | |
| } | |
| return updated; | |
| }); | |
| } catch (e) { | |
| console.warn('Error handling cursor-result message:', e); | |
| } | |
| } | |
| // Store session ID for future use and trigger refresh | |
| if (cursorSessionId && !currentSessionId) { | |
| setCurrentSessionId(cursorSessionId); | |
| case 'cursor-result': { | |
| // Get session ID from message or fall back to current session | |
| const cursorCompletedSessionId = latestMessage.sessionId || currentSessionId; | |
| // Only update UI state if this is the current session | |
| if (cursorCompletedSessionId === currentSessionId) { | |
| setIsLoading(false); | |
| setCanAbortSession(false); | |
| setClaudeStatus(null); | |
| } | |
| // Always mark the completed session as inactive and not processing | |
| if (cursorCompletedSessionId) { | |
| if (onSessionInactive) { | |
| onSessionInactive(cursorCompletedSessionId); | |
| } | |
| if (onSessionNotProcessing) { | |
| onSessionNotProcessing(cursorCompletedSessionId); | |
| } | |
| } | |
| // Only process result for current session | |
| if (cursorCompletedSessionId === currentSessionId) { | |
| try { | |
| const r = latestMessage.data || {}; | |
| const textResult = typeof r.result === 'string' ? r.result : ''; | |
| // Flush buffered deltas before finalizing | |
| if (streamTimerRef.current) { | |
| clearTimeout(streamTimerRef.current); | |
| streamTimerRef.current = null; | |
| } | |
| const pendingChunk = streamBufferRef.current; | |
| streamBufferRef.current = ''; | |
| setChatMessages(prev => { | |
| const updated = [...prev]; | |
| // Try to consolidate into the last streaming assistant message | |
| const last = updated[updated.length - 1]; | |
| if (last && last.type === 'assistant' && !last.isToolUse && last.isStreaming) { | |
| // Replace streaming content with the final content so deltas don't remain | |
| const finalContent = textResult && textResult.trim() | |
| ? textResult | |
| : (last.content || '') + (pendingChunk || ''); | |
| last.content = finalContent; | |
| last.isStreaming = false; | |
| } else if (textResult && textResult.trim()) { | |
| updated.push({ | |
| type: r.is_error ? 'error' : 'assistant', | |
| content: textResult, | |
| timestamp: new Date(), | |
| isStreaming: false | |
| }); | |
| } | |
| return updated; | |
| }); | |
| } catch (e) { | |
| console.warn('Error handling cursor-result message:', e); | |
| } | |
| } | |
| break; | |
| } |
🧰 Tools
🪛 Biome (2.1.2)
[error] 2673-2675: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🤖 Prompt for AI Agents
In src/components/ChatInterface.jsx around lines 2671 to 2723, the `case
'cursor-result'` declares constants that escape the switch scope and trigger the
noSwitchDeclarations lint rule; wrap the entire case body in a block by adding
an opening brace immediately after the `case 'cursor-result':` and a closing
brace before the `break;`, ensuring all local variables (e.g.,
cursorCompletedSessionId, r, textResult, pendingChunk) are scoped to that block
and then add an explicit `break;` after the closing brace to prevent
fall-through.
| case 'claude-complete': | ||
| setIsLoading(false); | ||
| setCanAbortSession(false); | ||
| setClaudeStatus(null); | ||
| // Get session ID from message or fall back to current session | ||
| const completedSessionId = latestMessage.sessionId || currentSessionId || sessionStorage.getItem('pendingSessionId'); | ||
|
|
||
|
|
||
| // Session Protection: Mark session as inactive to re-enable automatic project updates | ||
| // Conversation is complete, safe to allow project updates again | ||
| // Use real session ID if available, otherwise use pending session ID | ||
| const activeSessionId = currentSessionId || sessionStorage.getItem('pendingSessionId'); | ||
| if (activeSessionId && onSessionInactive) { | ||
| onSessionInactive(activeSessionId); | ||
| // Only update UI state if this is the current session | ||
| if (completedSessionId === currentSessionId) { | ||
| setIsLoading(false); | ||
| setCanAbortSession(false); | ||
| setClaudeStatus(null); | ||
| } | ||
|
|
||
| // Always mark the completed session as inactive and not processing | ||
| if (completedSessionId) { | ||
| if (onSessionInactive) { | ||
| onSessionInactive(completedSessionId); | ||
| } | ||
| if (onSessionNotProcessing) { | ||
| onSessionNotProcessing(completedSessionId); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Wrap case declarations in a block (noSwitchDeclarations).
The claude-complete case has the same issue as cursor-result - declarations need to be scoped to the case block.
Apply this diff:
- case 'claude-complete':
- // Get session ID from message or fall back to current session
- const completedSessionId = latestMessage.sessionId || currentSessionId || sessionStorage.getItem('pendingSessionId');
-
+ case 'claude-complete': {
+ const completedSessionId = latestMessage.sessionId || currentSessionId || sessionStorage.getItem('pendingSessionId');
+
// ... rest of the case logic ...
break;
+ }📝 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.
| case 'claude-complete': | |
| setIsLoading(false); | |
| setCanAbortSession(false); | |
| setClaudeStatus(null); | |
| // Get session ID from message or fall back to current session | |
| const completedSessionId = latestMessage.sessionId || currentSessionId || sessionStorage.getItem('pendingSessionId'); | |
| // Session Protection: Mark session as inactive to re-enable automatic project updates | |
| // Conversation is complete, safe to allow project updates again | |
| // Use real session ID if available, otherwise use pending session ID | |
| const activeSessionId = currentSessionId || sessionStorage.getItem('pendingSessionId'); | |
| if (activeSessionId && onSessionInactive) { | |
| onSessionInactive(activeSessionId); | |
| // Only update UI state if this is the current session | |
| if (completedSessionId === currentSessionId) { | |
| setIsLoading(false); | |
| setCanAbortSession(false); | |
| setClaudeStatus(null); | |
| } | |
| // Always mark the completed session as inactive and not processing | |
| if (completedSessionId) { | |
| if (onSessionInactive) { | |
| onSessionInactive(completedSessionId); | |
| } | |
| if (onSessionNotProcessing) { | |
| onSessionNotProcessing(completedSessionId); | |
| } | |
| } | |
| case 'claude-complete': { | |
| const completedSessionId = latestMessage.sessionId | |
| || currentSessionId | |
| || sessionStorage.getItem('pendingSessionId'); | |
| // Only update UI state if this is the current session | |
| if (completedSessionId === currentSessionId) { | |
| setIsLoading(false); | |
| setCanAbortSession(false); | |
| setClaudeStatus(null); | |
| } | |
| // Always mark the completed session as inactive and not processing | |
| if (completedSessionId) { | |
| if (onSessionInactive) { | |
| onSessionInactive(completedSessionId); | |
| } | |
| if (onSessionNotProcessing) { | |
| onSessionNotProcessing(completedSessionId); | |
| } | |
| } | |
| break; | |
| } |
🧰 Tools
🪛 Biome (2.1.2)
[error] 2770-2772: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🤖 Prompt for AI Agents
In src/components/ChatInterface.jsx around lines 2768 to 2787, the `case
'claude-complete':` declares a const (completedSessionId) at case scope which
violates noSwitchDeclarations; wrap the case body in a block by adding an
opening brace after the case label and a closing brace before the `break;`, so
all declarations (const completedSessionId and any others) are properly scoped
to that block and add/keep the `break` after the block.
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 (3)
src/components/ChatInterface.jsx (3)
2387-2569: Wrap case 'claude-response' declarations in a block.The
messageDataconst at line 2388 should be block-scoped to prevent access from other case clauses.Apply this diff:
- case 'claude-response': - const messageData = latestMessage.data.message || latestMessage.data; + case 'claude-response': { + const messageData = latestMessage.data.message || latestMessage.data; // ... existing logic ... break; + }
2671-2735: Wrap case 'cursor-result' declarations in a block.The
cursorCompletedSessionIdandpendingCursorSessionIdconsts should be block-scoped to prevent access from other case clauses.Apply this diff:
- case 'cursor-result': - // Get session ID from message or fall back to current session - const cursorCompletedSessionId = latestMessage.sessionId || currentSessionId; + case 'cursor-result': { + const cursorCompletedSessionId = latestMessage.sessionId || currentSessionId; // ... existing logic ... - const pendingCursorSessionId = sessionStorage.getItem('pendingSessionId'); + const pendingCursorSessionId = sessionStorage.getItem('pendingSessionId'); // ... more logic ... break; + }
2768-2805: Wrap case 'claude-complete' declarations in a block.The
completedSessionIdandpendingSessionIdconsts should be block-scoped to prevent access from other case clauses.Apply this diff:
- case 'claude-complete': - // Get session ID from message or fall back to current session - const completedSessionId = latestMessage.sessionId || currentSessionId || sessionStorage.getItem('pendingSessionId'); + case 'claude-complete': { + const completedSessionId = latestMessage.sessionId || currentSessionId || sessionStorage.getItem('pendingSessionId'); // ... existing logic ... - const pendingSessionId = sessionStorage.getItem('pendingSessionId'); + const pendingSessionId = sessionStorage.getItem('pendingSessionId'); // ... more logic ... break; + }
♻️ Duplicate comments (2)
server/index.js (2)
415-429: Fix path traversal check to prevent sibling directory access.Same issue as the file read endpoint:
startsWithcan be bypassed by sibling paths.Apply the same fix as suggested for lines 377-394:
- const resolved = path.resolve(filePath); - const normalizedRoot = path.resolve(projectRoot) + path.sep; - if (!resolved.startsWith(normalizedRoot)) { - return res.status(403).json({ error: 'Path must be under project root' }); - } + const resolved = path.resolve(filePath); + const normalizedRoot = path.resolve(projectRoot); + const rel = path.relative(normalizedRoot, resolved); + if (rel.startsWith('..') || path.isAbsolute(rel)) { + return res.status(403).json({ error: 'Path must be under project root' }); + }
469-487: Fix path traversal check to prevent sibling directory access.Same
startsWithvulnerability as the read endpoints, but more critical since this endpoint allows file writes.Apply the same
path.relativefix:- const resolved = path.resolve(filePath); - const normalizedRoot = path.resolve(projectRoot) + path.sep; - if (!resolved.startsWith(normalizedRoot)) { - return res.status(403).json({ error: 'Path must be under project root' }); - } + const resolved = path.resolve(filePath); + const normalizedRoot = path.resolve(projectRoot); + const rel = path.relative(normalizedRoot, resolved); + if (rel.startsWith('..') || path.isAbsolute(rel)) { + return res.status(403).json({ error: 'Path must be under project root' }); + }
🧹 Nitpick comments (1)
server/utils/commandParser.js (1)
46-72: Consider escaping special characters in replacement values.If arguments contain
$or&, these have special meaning in regex replacement strings and may not be replaced as expected. For example, if an argument is"$100", the replacement might behave unexpectedly.Use a replacement function to avoid special character interpretation:
// Replace positional arguments $1-$9 for (let i = 1; i <= 9; i++) { const regex = new RegExp(`\\$${i}`, 'g'); const value = argsArray[i - 1] || ''; - result = result.replace(regex, value); + result = result.replace(regex, () => value); }Similarly for
$ARGUMENTS:// Replace $ARGUMENTS with all arguments joined by space const allArgs = argsArray.join(' '); - result = result.replace(/\$ARGUMENTS/g, allArgs); + result = result.replace(/\$ARGUMENTS/g, () => allArgs);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
server/index.js(10 hunks)server/utils/commandParser.js(1 hunks)src/components/ChatInterface.jsx(30 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
server/index.js (4)
server/projects.js (7)
projectName(900-900)projectDir(271-271)projectDir(525-525)projectDir(714-714)projectDir(801-801)projectDir(864-864)projectDir(904-904)server/routes/commands.js (2)
sessionId(221-221)homeDir(394-394)server/claude-sdk.js (7)
mimeType(209-209)activeSessions(21-21)contextWindow(170-170)inputTokens(163-163)cacheCreationTokens(166-166)cacheReadTokens(165-165)totalUsed(169-169)server/cursor-cli.js (1)
process(239-239)
src/components/ChatInterface.jsx (4)
src/App.jsx (2)
showThinking(60-60)processingSessions(71-71)src/utils/api.js (3)
authenticatedFetch(2-20)authenticatedFetch(2-20)url(53-53)server/routes/commands.js (5)
allCommands(380-380)result(492-492)content(39-39)content(448-448)content(533-533)src/components/CommandMenu.jsx (1)
CommandMenu(13-292)
server/utils/commandParser.js (1)
server/routes/commands.js (8)
content(39-39)content(448-448)content(533-533)result(492-492)resolvedPath(438-438)resolvedPath(517-517)processedContent(536-536)commandName(45-45)
🪛 Biome (2.1.2)
src/components/ChatInterface.jsx
[error] 2388-2390: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2673-2675: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2725-2726: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2770-2772: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (12)
server/index.js (4)
583-656: LGTM! WebSocket integration with Claude SDK is well-structured.The provider-based routing and session management handlers are correctly implemented with proper delegation to Claude SDK and Cursor CLI functions.
1123-1213: LGTM! Token usage endpoint is well-implemented with proper security.The endpoint correctly:
- Uses
path.relativefor traversal protection (unlike the file endpoints above)- Handles async I/O properly
- Sanitizes sessionId against dangerous characters
- Parses token usage from the latest assistant message
- Falls back gracefully when CONTEXT_WINDOW is invalid
1216-1224: LGTM! Standard Vite port is now used.The fallback to port 5173 aligns with Vite's default configuration.
1317-1324: LGTM! Startup flow properly initializes watcher and logs SDK usage.The async watcher setup and informative logging are appropriate.
server/utils/commandParser.js (7)
1-26: LGTM! Secure imports and reasonable configuration.The use of
execFile(notexec) andshell-quotefor parsing demonstrates good security practices. The allowlist includes common safe commands, and the configuration constants are appropriate.
28-44: LGTM! Straightforward frontmatter parsing with proper error handling.The function correctly uses
gray-matterand provides structured output.
74-89: LGTM! Path traversal check correctly implemented.The use of
path.relativewith three safety checks (empty string,..prefix, absolute path) correctly prevents directory traversal attacks. This matches the pattern used in the token-usage endpoint.
91-143: LGTM! File include processing is secure and well-structured.The function correctly:
- Limits recursion depth to prevent infinite loops
- Validates paths with
isPathSafebefore reading- Recursively processes nested includes
- Handles file-not-found errors gracefully
- Preserves formatting by maintaining leading spaces
145-211: LGTM! Command validation is comprehensive and secure.The function correctly:
- Parses commands with
shell-quoteto handle quotes properly- Rejects shell operators to prevent command chaining
- Uses exact allowlist matching (no dangerous prefix matching)
- Validates arguments against a comprehensive dangerous character pattern
- Returns detailed error information for debugging
This implementation follows security best practices for command execution.
213-222: LGTM! Backward compatibility wrapper is appropriate.The deprecated function correctly wraps
validateCommandwhile clearly indicating users should migrate to the new API.
224-303: LGTM! Output sanitization and command execution are secure.Both functions are well-implemented:
sanitizeOutput:
- Correctly filters control characters using charCode checks
- Preserves essential whitespace (tab, newline, carriage return)
- Avoids regex control character issues flagged by linters
processBashCommands:
- Validates commands before execution using
validateCommand- Uses
execFilewithshell: falseto prevent shell injection- Enforces timeout and buffer limits
- Provides clear error messages for timeouts and failures
- Sanitizes all output before returning
The security measures throughout are exemplary.
src/components/ChatInterface.jsx (1)
3396-3433: Verify intended command execution behavior.There's an inconsistency in command selection:
- Keyboard selection (Tab/Enter at lines 3452-3459): Calls
selectCommand, which only inserts the command into input without executing (lines 3405-3406)- Mouse click in CommandMenu: Calls
handleCommandSelectwithisHover=false(line 4064), which executes the command immediately (line 1424)This means clicking a command executes it right away, but pressing Tab/Enter just inserts it for the user to review and manually send.
Question: Is this intentional? If commands should only be inserted (not auto-executed), remove the execution from
handleCommandSelect. If commands should auto-execute on selection, add execution toselectCommandas well.Based on the past review comment that said "Don't execute on selection; only insert the command," it appears auto-execution should be removed from
handleCommandSelect:const handleCommandSelect = useCallback((command, index, isHover) => { if (!command || !selectedProject) return; // If hovering, just update the selected index if (isHover) { setSelectedCommandIndex(index); return; } // Update command history const historyKey = `command_history_${selectedProject.name}`; // ... history tracking code ... parsedHistory[command.name] = (parsedHistory[command.name] || 0) + 1; safeLocalStorage.setItem(historyKey, JSON.stringify(parsedHistory)); - - // Execute the command - executeCommand(command); }, [selectedProject]);Then update the click handler in CommandMenu to call
selectCommandinstead:- onSelect={handleCommandSelect} + onSelect={(command, index, isHover) => { + if (isHover) { + setSelectedCommandIndex(index); + } else { + selectCommand(command); + // Track usage + const historyKey = `command_history_${selectedProject.name}`; + // ... history tracking ... + } + }}Also applies to: 1399-1425, 4060-4081
| // Security: ensure the requested path is inside the project root | ||
| if (!filePath) { | ||
| return res.status(400).json({ error: 'Invalid file path' }); | ||
| } | ||
|
|
||
| const content = await fsPromises.readFile(filePath, 'utf8'); | ||
| res.json({ content, path: filePath }); | ||
| const projectRoot = await extractProjectDirectory(projectName).catch(() => null); | ||
| if (!projectRoot) { | ||
| return res.status(404).json({ error: 'Project not found' }); | ||
| } | ||
|
|
||
| const resolved = path.resolve(filePath); | ||
| const normalizedRoot = path.resolve(projectRoot) + path.sep; | ||
| if (!resolved.startsWith(normalizedRoot)) { | ||
| return res.status(403).json({ error: 'Path must be under project root' }); | ||
| } | ||
|
|
||
| const content = await fsPromises.readFile(resolved, 'utf8'); | ||
| res.json({ content, path: resolved }); |
There was a problem hiding this comment.
Fix path traversal check to prevent sibling directory access.
The current check uses startsWith which can be bypassed by sibling paths. For example, if normalizedRoot is /home/user/project, a path like /home/user/project-evil/file would pass the check.
Use path.relative like the token-usage endpoint does (lines 1152-1155):
- const resolved = path.resolve(filePath);
- const normalizedRoot = path.resolve(projectRoot) + path.sep;
- if (!resolved.startsWith(normalizedRoot)) {
- return res.status(403).json({ error: 'Path must be under project root' });
- }
+ const resolved = path.resolve(filePath);
+ const normalizedRoot = path.resolve(projectRoot);
+ const rel = path.relative(normalizedRoot, resolved);
+ if (rel.startsWith('..') || path.isAbsolute(rel)) {
+ return res.status(403).json({ error: 'Path must be under project root' });
+ }📝 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.
| // Security: ensure the requested path is inside the project root | |
| if (!filePath) { | |
| return res.status(400).json({ error: 'Invalid file path' }); | |
| } | |
| const content = await fsPromises.readFile(filePath, 'utf8'); | |
| res.json({ content, path: filePath }); | |
| const projectRoot = await extractProjectDirectory(projectName).catch(() => null); | |
| if (!projectRoot) { | |
| return res.status(404).json({ error: 'Project not found' }); | |
| } | |
| const resolved = path.resolve(filePath); | |
| const normalizedRoot = path.resolve(projectRoot) + path.sep; | |
| if (!resolved.startsWith(normalizedRoot)) { | |
| return res.status(403).json({ error: 'Path must be under project root' }); | |
| } | |
| const content = await fsPromises.readFile(resolved, 'utf8'); | |
| res.json({ content, path: resolved }); | |
| // Security: ensure the requested path is inside the project root | |
| if (!filePath) { | |
| return res.status(400).json({ error: 'Invalid file path' }); | |
| } | |
| const projectRoot = await extractProjectDirectory(projectName).catch(() => null); | |
| if (!projectRoot) { | |
| return res.status(404).json({ error: 'Project not found' }); | |
| } | |
| - const resolved = path.resolve(filePath); | |
| - const normalizedRoot = path.resolve(projectRoot) + path.sep; | |
| - if (!resolved.startsWith(normalizedRoot)) { | |
| - return res.status(403).json({ error: 'Path must be under project root' }); | |
| const resolved = path.resolve(filePath); | |
| const normalizedRoot = path.resolve(projectRoot); | |
| const rel = path.relative(normalizedRoot, resolved); | |
| if (rel.startsWith('..') || path.isAbsolute(rel)) { | |
| return res.status(403).json({ error: 'Path must be under project root' }); | |
| } | |
| const content = await fsPromises.readFile(resolved, 'utf8'); | |
| res.json({ content, path: resolved }); |
🤖 Prompt for AI Agents
In server/index.js around lines 377 to 394, the current path traversal
protection uses startsWith and can be bypassed by sibling directories (e.g.,
/home/user/project-evil). Replace the startsWith logic with a
path.relative-based check: compute the relative path from the resolved project
root to the requested resolved path, and reject the request if the relative path
begins with '..' or is an absolute path (indicating the file is outside the
project root); only proceed to read the file after this check passes. Ensure you
use path.resolve(projectRoot) and path.resolve(filePath) for both sides before
computing relative to avoid spoofing via .. segments.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
server/index.js(14 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
server/index.js (3)
server/projects.js (10)
projectName(900-900)content(683-683)content(814-814)projectDir(271-271)projectDir(525-525)projectDir(714-714)projectDir(801-801)projectDir(864-864)projectDir(904-904)lines(815-815)server/routes/commands.js (5)
content(39-39)content(448-448)content(533-533)homeDir(394-394)__dirname(9-9)server/claude-sdk.js (7)
mimeType(209-209)activeSessions(21-21)contextWindow(170-170)inputTokens(163-163)cacheCreationTokens(166-166)cacheReadTokens(165-165)totalUsed(169-169)
🔇 Additional comments (6)
server/index.js (6)
30-31: LGTM: Import and route registration changes.The new imports for WebSocket, Claude SDK, Cursor CLI, and Commands routes are correctly structured and follow the existing patterns in the codebase.
Also applies to: 41-42, 49-49, 196-198
111-111: LGTM: WebSocket readyState constant usage.Using the
WebSocket.OPENconstant instead of magic numbers improves code clarity and follows best practices.Also applies to: 784-784, 831-831, 868-868
583-655: LGTM: Claude SDK integration.The migration from CLI-based interactions to Claude SDK is well-structured, with proper provider-based routing and error handling in place.
1123-1213: Verify token calculation aligns with requirements.The endpoint calculates
totalUsed = inputTokens + cacheCreationTokens + cacheReadTokens, intentionally excludingoutput_tokens(line 1198). This differs from the SDK implementation (claude-sdk.js line 168), which includes output tokens.The exclusion appears intentional for measuring context window consumption (input only) rather than total API usage. If this is the intended behavior, the implementation is correct. However, verify this aligns with the product requirements.
For reference, if you need to match the SDK's total usage calculation, apply:
- // Calculate total context usage (excluding output_tokens, as per ccusage) - const totalUsed = inputTokens + cacheCreationTokens + cacheReadTokens; + // Calculate total token usage (including output tokens) + let outputTokens = 0; + // Re-scan for output_tokens from the same message + for (let i = lines.length - 1; i >= 0; i--) { + try { + const entry = JSON.parse(lines[i]); + if (entry.type === 'assistant' && entry.message?.usage) { + outputTokens = entry.message.usage.output_tokens || 0; + break; + } + } catch { continue; } + } + const totalUsed = inputTokens + outputTokens + cacheCreationTokens + cacheReadTokens;
1215-1225: LGTM: Routing improvements.The updated comments and fallback port for Vite development server improve code clarity and robustness.
1317-1324: LGTM: Startup logging enhancement.The added logging clearly indicates the Claude Agents SDK integration mode, improving operational visibility.
| // Security: ensure the requested path is inside the project root | ||
| if (!filePath) { | ||
| return res.status(400).json({ error: 'Invalid file path' }); | ||
| } | ||
|
|
||
| const content = await fsPromises.readFile(filePath, 'utf8'); | ||
| res.json({ content, path: filePath }); | ||
| const projectRoot = await extractProjectDirectory(projectName).catch(() => null); | ||
| if (!projectRoot) { | ||
| return res.status(404).json({ error: 'Project not found' }); | ||
| } | ||
|
|
||
| const resolved = path.resolve(filePath); | ||
| const normalizedRoot = path.resolve(projectRoot) + path.sep; | ||
| if (!resolved.startsWith(normalizedRoot)) { | ||
| return res.status(403).json({ error: 'Path must be under project root' }); | ||
| } | ||
|
|
||
| const content = await fsPromises.readFile(resolved, 'utf8'); | ||
| res.json({ content, path: resolved }); |
There was a problem hiding this comment.
Symlink vulnerability: resolve symlinks before path validation.
The current security check uses path.resolve() which doesn't follow symlinks. A malicious symlink placed inside the project root could point to files outside the project, bypassing the prefix check.
Apply this approach to fix the vulnerability:
const projectRoot = await extractProjectDirectory(projectName).catch(() => null);
if (!projectRoot) {
return res.status(404).json({ error: 'Project not found' });
}
- const resolved = path.resolve(filePath);
+ // Resolve symlinks to prevent escape via symlink
+ let resolved;
+ try {
+ resolved = await fsPromises.realpath(filePath);
+ } catch (error) {
+ if (error.code === 'ENOENT') {
+ return res.status(404).json({ error: 'File not found' });
+ }
+ throw error;
+ }
const normalizedRoot = path.resolve(projectRoot) + path.sep;
if (!resolved.startsWith(normalizedRoot)) {
return res.status(403).json({ error: 'Path must be under project root' });
}Apply the same pattern to the binary file serve endpoint (lines 425-429) and file save endpoint (lines 483-487).
📝 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.
| // Security: ensure the requested path is inside the project root | |
| if (!filePath) { | |
| return res.status(400).json({ error: 'Invalid file path' }); | |
| } | |
| const content = await fsPromises.readFile(filePath, 'utf8'); | |
| res.json({ content, path: filePath }); | |
| const projectRoot = await extractProjectDirectory(projectName).catch(() => null); | |
| if (!projectRoot) { | |
| return res.status(404).json({ error: 'Project not found' }); | |
| } | |
| const resolved = path.resolve(filePath); | |
| const normalizedRoot = path.resolve(projectRoot) + path.sep; | |
| if (!resolved.startsWith(normalizedRoot)) { | |
| return res.status(403).json({ error: 'Path must be under project root' }); | |
| } | |
| const content = await fsPromises.readFile(resolved, 'utf8'); | |
| res.json({ content, path: resolved }); | |
| // Security: ensure the requested path is inside the project root | |
| if (!filePath) { | |
| return res.status(400).json({ error: 'Invalid file path' }); | |
| } | |
| const projectRoot = await extractProjectDirectory(projectName).catch(() => null); | |
| if (!projectRoot) { | |
| return res.status(404).json({ error: 'Project not found' }); | |
| } | |
| // Resolve symlinks to prevent escape via symlink | |
| let resolved; | |
| try { | |
| resolved = await fsPromises.realpath(filePath); | |
| } catch (error) { | |
| if (error.code === 'ENOENT') { | |
| return res.status(404).json({ error: 'File not found' }); | |
| } | |
| throw error; | |
| } | |
| const normalizedRoot = path.resolve(projectRoot) + path.sep; | |
| if (!resolved.startsWith(normalizedRoot)) { | |
| return res.status(403).json({ error: 'Path must be under project root' }); | |
| } | |
| const content = await fsPromises.readFile(resolved, 'utf8'); | |
| res.json({ content, path: resolved }); |
🤖 Prompt for AI Agents
In server/index.js around lines 377-394, the code validates paths using
path.resolve which does not follow symlinks, allowing a symlink inside the
project to point outside and bypass the check; replace the current check by
resolving real filesystem paths with fsPromises.realpath for both the project
root and the requested file (or their resolved targets), then compare using the
real project root + path.sep as a prefix against the real requested path and
reject if it doesn't start with that prefix; apply the same realpath-based
validation pattern to the binary serve endpoint (lines ~425-429) and the file
save endpoint (lines ~483-487), and ensure proper error handling for realpath
failures (treat as invalid/forbidden) before reading, serving, or writing files.
- Use cumulative tokens from SDK instead of per-request tokens for accurate session totals - Add configurable context window budget via CONTEXT_WINDOW env var (default 160000) - Fix race condition where stale token usage data could overwrite current session data - Replace polling with one-time fetch on session load + post-message update - Add comprehensive debug logging for token budget flow - Show token percentage on all screen sizes (remove sm:inline hiding) - Add .mcp.json to .gitignore - Add ARCHITECTURE.md and slash-command-tasks.md documentation Technical improvements: - Token budget now fetched after message completion instead of WebSocket - Removed interval polling that could conflict with WebSocket updates - Added session/project validation before updating state - Improved input placeholder to wrap on small screens 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/components/ChatInterface.jsx (1)
1399-1426: Command menu clicks still execute immediately.
handleCommandSelectis invoked on item clicks and still callsexecuteCommand, so mouse selection fires the command right away. Keyboard selection routes throughselectCommand, which only inserts the slash command. The two paths now disagree, which reintroduces the bug where clicking a menu item unexpectedly runs the command and clears the input. Please make mouse selection match the keyboard path by only inserting the command (and letting the user confirm). For example:- // Update command history - const historyKey = `command_history_${selectedProject.name}`; - ... - parsedHistory[command.name] = (parsedHistory[command.name] || 0) + 1; - safeLocalStorage.setItem(historyKey, JSON.stringify(parsedHistory)); - - // Execute the command - executeCommand(command); + // Update history for actual selections + const historyKey = `command_history_${selectedProject.name}`; + ... + parsedHistory[command.name] = (parsedHistory[command.name] || 0) + 1; + safeLocalStorage.setItem(historyKey, JSON.stringify(parsedHistory)); + + selectCommand(command);Keep the hover branch as-is so the keyboard index follows the cursor.
🧹 Nitpick comments (3)
server/claude-sdk.js (1)
176-229: Avoid logging computed token budgets in production.
extractTokenBudgetemits raw token usage to stdout every query. This function runs for every streamed result, so the console spam is noisy and risks exposing usage data in shared logs. Please drop theseconsole.logcalls or gate them behind a debug flag.src/components/ChatInterface.jsx (1)
1218-1235: Clean up the slash-command debounce timer on unmount.
commandQueryTimerRefcan leave a pending timeout firing after the component unmounts. Add a one-timeuseEffectcleanup that clears the timer to avoid stray state updates.const commandQueryTimerRef = useRef(null); +useEffect(() => { + return () => { + if (commandQueryTimerRef.current) { + clearTimeout(commandQueryTimerRef.current); + } + }; +}, []);slash-command-tasks.md (1)
63-76: Document the confirm-before-execute behavior.Steps 6–7 still say the UI should execute the command immediately after selection. That conflicts with the intended UX (selection inserts the command and the user triggers execution). Please rewrite these steps to describe inserting the command into the input and letting the send flow run it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.gitignore(2 hunks)ARCHITECTURE.md(1 hunks)server/claude-sdk.js(1 hunks)slash-command-tasks.md(1 hunks)src/components/ChatInterface.jsx(30 hunks)src/components/TokenUsagePie.jsx(1 hunks)src/index.css(1 hunks)src/utils/websocket.js(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/index.css
🚧 Files skipped from review as they are similar to previous changes (2)
- .gitignore
- src/components/TokenUsagePie.jsx
🧰 Additional context used
🧬 Code graph analysis (2)
src/components/ChatInterface.jsx (3)
src/App.jsx (6)
showThinking(60-60)selectedProject(49-49)selectedSession(50-50)processingSessions(71-71)isInputFocused(55-55)sendByCtrlEnter(62-62)src/utils/api.js (3)
authenticatedFetch(2-20)authenticatedFetch(2-20)url(53-53)src/components/CommandMenu.jsx (1)
CommandMenu(13-292)
server/claude-sdk.js (6)
src/components/ChatInterface.jsx (2)
permissionMode(1209-1209)tokenBudget(1235-1235)server/routes/commands.js (1)
sessionId(221-221)src/components/Sidebar.jsx (1)
getAllSessions(336-343)server/index.js (9)
inputTokens(1171-1171)cacheReadTokens(1173-1173)cacheCreationTokens(1172-1172)totalUsed(1198-1198)contextWindow(1170-1170)path(1042-1042)fs(1043-1043)i(1176-1176)os(1044-1044)src/components/ImageViewer.jsx (1)
imagePath(6-6)src/utils/websocket.js (1)
ws(4-4)
🪛 Biome (2.1.2)
src/components/ChatInterface.jsx
[error] 2389-2391: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2674-2676: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2726-2727: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2771-2773: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🪛 LanguageTool
ARCHITECTURE.md
[grammar] ~3-~3: There might be a mistake here.
Context: ...ure Documentation Generated: 2025-10-13 Purpose: Foundation for slash commands i...
(QB_NEW_EN)
[grammar] ~9-~9: There might be a mistake here.
Context: ...s** with ES Modules ("type": "module") - Express.js v4.18.2 - Web framework - *...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...- Express.js v4.18.2 - Web framework - WebSocket (ws) v8.14.2 - Real-time com...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ...ramework - WebSocket (ws) v8.14.2 - Real-time communication - Better-SQLite3 ...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ...(ws)** v8.14.2 - Real-time communication - Better-SQLite3 v12.2.0 - Session/messa...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...** v12.2.0 - Session/message persistence - Authentication: JWT (jsonwebtoken v9.0...
(QB_NEW_EN)
[grammar] ~13-~13: There might be a mistake here.
Context: ...WT (jsonwebtoken v9.0.2) + bcrypt v6.0.0 - Process Management: - node-pty v1.1....
(QB_NEW_EN)
[grammar] ~14-~14: There might be a mistake here.
Context: ... bcrypt v6.0.0 - Process Management: - node-pty v1.1.0-beta34 (terminal emulati...
(QB_NEW_EN)
[grammar] ~15-~15: There might be a mistake here.
Context: ...e-pty v1.1.0-beta34 (terminal emulation) - cross-spawn v7.0.3 (process spawning) - ...
(QB_NEW_EN)
[grammar] ~16-~16: There might be a mistake here.
Context: ... - cross-spawn v7.0.3 (process spawning) - Claude SDK: @anthropic-ai/claude-agent...
(QB_NEW_EN)
[grammar] ~19-~19: There might be a mistake here.
Context: ...i/claude-agent-sdk v0.1.13 ### Frontend - React v18.2.0 - Build Tool: Vite v...
(QB_NEW_EN)
[grammar] ~20-~20: There might be a mistake here.
Context: ...0.1.13 ### Frontend - React v18.2.0 - Build Tool: Vite v7.0.4 - Styling:...
(QB_NEW_EN)
[grammar] ~21-~21: There might be a mistake here.
Context: ...** v18.2.0 - Build Tool: Vite v7.0.4 - Styling: TailwindCSS v3.4.0 + @Tailwin...
(QB_NEW_EN)
[grammar] ~22-~22: There might be a mistake here.
Context: ...windCSS v3.4.0 + @tailwindcss/typography - Routing: react-router-dom v6.8.1 - **C...
(QB_NEW_EN)
[grammar] ~23-~23: There might be a mistake here.
Context: ...y - Routing: react-router-dom v6.8.1 - Code Editor: @uiw/react-codemirror v4....
(QB_NEW_EN)
[grammar] ~24-~24: There might be a mistake here.
Context: ...Editor**: @uiw/react-codemirror v4.23.13 - Markdown: react-markdown v10.1.0 - **F...
(QB_NEW_EN)
[grammar] ~25-~25: There might be a mistake here.
Context: ...3 - Markdown: react-markdown v10.1.0 - File Upload: react-dropzone v14.2.3 - ...
(QB_NEW_EN)
[grammar] ~26-~26: There might be a mistake here.
Context: ... File Upload: react-dropzone v14.2.3 - Terminal: @xterm/xterm v5.5.0 with Web...
(QB_NEW_EN)
[grammar] ~66-~66: There might be a mistake here.
Context: ...outes) ### Authentication (/api/auth) - POST /api/auth/register - User registr...
(QB_NEW_EN)
[grammar] ~67-~67: There might be a mistake here.
Context: .../api/auth/register - User registration - POST /api/auth/login - User login - Al...
(QB_NEW_EN)
[grammar] ~68-~68: There might be a mistake here.
Context: ...on - POST /api/auth/login - User login - All other endpoints require JWT authenti...
(QB_NEW_EN)
[grammar] ~71-~71: There might be a mistake here.
Context: ...tication ### Projects (/api/projects) - GET /api/projects - List all projects ...
(QB_NEW_EN)
[grammar] ~72-~72: There might be a mistake here.
Context: ... GET /api/projects - List all projects - GET `/api/projects/:projectName/sessions...
(QB_NEW_EN)
[grammar] ~73-~73: There might be a mistake here.
Context: ...ame/sessions- Get sessions for project - GET/api/projects/:projectName/sessions...
(QB_NEW_EN)
[grammar] ~74-~74: There might be a mistake here.
Context: ...ions/:sessionId/messages- Get messages - POST/api/projects/create` - Create new...
(QB_NEW_EN)
[grammar] ~75-~75: There might be a mistake here.
Context: ...pi/projects/create- Create new project - GET/api/projects/:projectName/file` - ...
(QB_NEW_EN)
[grammar] ~76-~76: There might be a mistake here.
Context: ...s/:projectName/file- Read file content - GET/api/projects/:projectName/files` -...
(QB_NEW_EN)
[grammar] ~77-~77: There might be a mistake here.
Context: ...:projectName/files- List project files - GET/api/projects/:projectName/files/co...
(QB_NEW_EN)
[grammar] ~78-~78: There might be a mistake here.
Context: ...Name/files/content- Serve binary files - POST/api/projects/:projectName/upload-...
(QB_NEW_EN)
[grammar] ~82-~82: There might be a mistake here.
Context: ...pi/config- Get WebSocket configuration - GET/api/browse-filesystem` - Browse fi...
(QB_NEW_EN)
[grammar] ~83-~83: There might be a mistake here.
Context: ... Browse filesystem for project selection - POST /api/transcribe - Audio transcrip...
(QB_NEW_EN)
[grammar] ~84-~84: There might be a mistake here.
Context: ... /api/transcribe - Audio transcription - GET `/api/sessions/:sessionId/token-usag...
(QB_NEW_EN)
[grammar] ~94-~94: There might be a mistake here.
Context: ...Architecture ### WebSocket Server Setup - Single WebSocket server handling multi...
(QB_NEW_EN)
[grammar] ~95-~95: There might be a mistake here.
Context: ...bSocket server** handling multiple paths - Authentication: Token-based via query ...
(QB_NEW_EN)
[grammar] ~96-~96: There might be a mistake here.
Context: ... Token-based via query params or headers - Port: Same as HTTP server (3001 defaul...
(QB_NEW_EN)
[grammar] ~97-~97: There might be a mistake here.
Context: ...rt**: Same as HTTP server (3001 default) - Connection verification in `verifyClie...
(QB_NEW_EN)
[grammar] ~103-~103: There might be a mistake here.
Context: ...e**: Main chat communication with Claude Handler: handleChatConnection(ws) ...
(QB_NEW_EN)
[grammar] ~116-~116: There might be a mistake here.
Context: ...om Claude - content: Message content - isComplete: Boolean for stream end - sessionId...
(QB_NEW_EN)
[grammar] ~117-~117: There might be a mistake here.
Context: ... - isComplete: Boolean for stream end - sessionId: Current session ID - `type: 'claude-co...
(QB_NEW_EN)
[grammar] ~126-~126: There might be a mistake here.
Context: ...erminal emulation for interactive shells Handler: handleShellConnection(ws)...
(QB_NEW_EN)
[grammar] ~129-~129: There might be a mistake here.
Context: ...andleShellConnection(ws)` Features: - Interactive terminal using node-pty - Su...
(QB_NEW_EN)
[grammar] ~130-~130: There might be a mistake here.
Context: ...*: - Interactive terminal using node-pty - Supports PowerShell (Windows) and bash/z...
(QB_NEW_EN)
[grammar] ~131-~131: There might be a mistake here.
Context: ...PowerShell (Windows) and bash/zsh (Unix) - Environment variable injection - Real-ti...
(QB_NEW_EN)
[grammar] ~132-~132: There might be a mistake here.
Context: ... (Unix) - Environment variable injection - Real-time input/output streaming ## Cha...
(QB_NEW_EN)
[grammar] ~182-~182: There might be a mistake here.
Context: ...andling Flow ### Current Implementation 1. Input Change: handleInputChange(e) ...
(QB_NEW_EN)
[grammar] ~183-~183: There might be a mistake here.
Context: ...ent Implementation 1. Input Change: handleInputChange(e) - Updates input state - Tracks cursor p...
(QB_NEW_EN)
[grammar] ~184-~184: There might be a mistake here.
Context: ...InputChange(e)` - Updates input state - Tracks cursor position - Handles @ fi...
(QB_NEW_EN)
[grammar] ~185-~185: There might be a mistake here.
Context: ... input state - Tracks cursor position - Handles @ file mentions (existing) - ...
(QB_NEW_EN)
[grammar] ~186-~186: There might be a mistake here.
Context: ... - Handles @ file mentions (existing) - Slash commands NOT implemented 2. **S...
(QB_NEW_EN)
[grammar] ~189-~189: There might be a mistake here.
Context: ...mands NOT implemented** 2. Submit: handleSubmit(e) - Uploads images if attached - Creates ...
(QB_NEW_EN)
[grammar] ~195-~195: There might be a mistake here.
Context: ...session as active 3. File Mention (@) - Detects @ symbol in input - Shows d...
(QB_NEW_EN)
[grammar] ~196-~196: There might be a mistake here.
Context: ...n (@)** - Detects @ symbol in input - Shows dropdown with project files - F...
(QB_NEW_EN)
[grammar] ~197-~197: There might be a mistake here.
Context: ...t - Shows dropdown with project files - Filters files by query - Keyboard nav...
(QB_NEW_EN)
[grammar] ~198-~198: There might be a mistake here.
Context: ...roject files - Filters files by query - Keyboard navigation (Arrow keys, Enter, ...
(QB_NEW_EN)
[grammar] ~203-~203: There might be a mistake here.
Context: ...uthentication Flow 1. User logs in via /api/auth/login 2. Server returns JWT token 3. Token stored...
(QB_NEW_EN)
[grammar] ~205-~205: There might be a mistake here.
Context: ...oken 3. Token stored in localStorage as 'auth-token' 4. All API requests include `Authorization:...
(QB_NEW_EN)
[grammar] ~219-~219: There might be a mistake here.
Context: ...lash Commands ### Required npm Packages - ✅ Already have: express, ws, react, reac...
(QB_NEW_EN)
[grammar] ~220-~220: There might be a mistake here.
Context: ...eady have: express, ws, react, react-dom - ❌ Need to add: gray-matter (for fron...
(QB_NEW_EN)
[grammar] ~221-~221: There might be a mistake here.
Context: ...for frontmatter parsing in .md commands) - ❌ Optional: fuse.js (for fuzzy searc...
(QB_NEW_EN)
[grammar] ~226-~226: There might be a mistake here.
Context: ...entions ### .claude/commands/ Structure - Project-level commands at `.claude/comma...
(QB_NEW_EN)
[grammar] ~227-227: There might be a mistake here./.claude/comman...
Context: .../ Structure - Project-level commands at .claude/commands/ - User-level commands at `
(QB_NEW_EN)
[grammar] ~228-228: There might be a mistake here./.claude/commands/
Context: ...ude/commands/- User-level commands at - Commands are markdown files (.md`) - Ca...
(QB_NEW_EN)
[grammar] ~250-~250: There might be a mistake here.
Context: ...n Points for Slash Commands ### Backend 1. New Router: `server/routes/commands.js...
(QB_NEW_EN)
[grammar] ~251-~251: There might be a mistake here.
Context: ...ommands ### Backend 1. New Router: server/routes/commands.js - Mount at /api/commands in server/index...
(QB_NEW_EN)
[grammar] ~255-~255: There might be a mistake here.
Context: ...authenticateToken 2. **New Utility**:server/utils/commandParser.js` - Parse markdown with gray-matter - Rep...
(QB_NEW_EN)
[grammar] ~261-~261: There might be a mistake here.
Context: ...ecute bash commands safely ### Frontend 3. ChatInterface.jsx Modifications - I...
(QB_NEW_EN)
[grammar] ~262-~262: There might be a mistake here.
Context: ...end 3. ChatInterface.jsx Modifications - Implement handleInputChange slash dete...
(QB_NEW_EN)
[grammar] ~264-~264: There might be a mistake here.
Context: ...ection - Fetch commands on mount via /api/commands/list - Filter commands as user types - Show ...
(QB_NEW_EN)
[grammar] ~269-~269: There might be a mistake here.
Context: ...and on selection 4. New Component: src/components/CommandMenu.jsx - Dropdown UI for command autocomplete ...
(QB_NEW_EN)
[grammar] ~270-~270: There might be a mistake here.
Context: ... - Dropdown UI for command autocomplete - Keyboard navigation (Arrow keys, Enter, ...
(QB_NEW_EN)
[grammar] ~271-~271: There might be a mistake here.
Context: ...igation (Arrow keys, Enter, Escape, Tab) - Mouse hover/click selection - Positio...
(QB_NEW_EN)
[grammar] ~272-~272: There might be a mistake here.
Context: ...e, Tab) - Mouse hover/click selection - Positioned near cursor ## Security Cons...
(QB_NEW_EN)
[grammar] ~277-~277: There might be a mistake here.
Context: ...ty Considerations ### Existing Security - JWT authentication on all API endpoints ...
(QB_NEW_EN)
[grammar] ~278-~278: There might be a mistake here.
Context: ... JWT authentication on all API endpoints - Token validation in WebSocket connection...
(QB_NEW_EN)
[grammar] ~279-~279: There might be a mistake here.
Context: ...oken validation in WebSocket connections - Path traversal prevention in file operat...
(QB_NEW_EN)
[grammar] ~280-~280: There might be a mistake here.
Context: ... traversal prevention in file operations - bcrypt password hashing ### Additional ...
(QB_NEW_EN)
[grammar] ~283-~283: There might be a mistake here.
Context: ...ord hashing ### Additional for Commands - Validate command paths (prevent director...
(QB_NEW_EN)
[grammar] ~293-~293: There might be a mistake here.
Context: ...ce command filtering (150ms recommended) - Cache parsed commands in memory - Virtua...
(QB_NEW_EN)
[grammar] ~294-~294: There might be a mistake here.
Context: ...ended) - Cache parsed commands in memory - Virtual scrolling for >20 commands (reac...
(QB_NEW_EN)
[grammar] ~295-~295: There might be a mistake here.
Context: ...crolling for >20 commands (react-window) - Lazy load command files (only parse on e...
(QB_NEW_EN)
[grammar] ~296-~296: There might be a mistake here.
Context: ... command files (only parse on execution) - LocalStorage cache for command list per ...
(QB_NEW_EN)
slash-command-tasks.md
[grammar] ~5-~5: There might be a mistake here.
Context: ...s.js) ### 1. Create Commands API Router - POST /api/commands/list: Scan and re...
(QB_NEW_EN)
[grammar] ~24-~24: There might be a mistake here.
Context: ...prompt ### 2. Built-in Commands Handler - Implement handlers for each built-in com...
(QB_NEW_EN)
[grammar] ~25-~25: There might be a mistake here.
Context: ...ment handlers for each built-in command: - /help: Return help documentation - /clear...
(QB_NEW_EN)
[grammar] ~26-~26: There might be a mistake here.
Context: ...: - /help: Return help documentation - /clear: Signal to clear chat history (frontend...
(QB_NEW_EN)
[grammar] ~27-~27: There might be a mistake here.
Context: ...to clear chat history (frontend handles) - /model: List/switch available models - `/cos...
(QB_NEW_EN)
[grammar] ~28-~28: There might be a mistake here.
Context: ...- /model: List/switch available models - /cost: Return current token usage stats - `...
(QB_NEW_EN)
[grammar] ~29-~29: There might be a mistake here.
Context: .../cost: Return current token usage stats - /memory: Open/edit CLAUDE.md - /config`: Ret...
(QB_NEW_EN)
[grammar] ~30-~30: There might be a mistake here.
Context: ...stats - /memory: Open/edit CLAUDE.md - /config: Return current config (frontend opens ...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ...current config (frontend opens settings) - /status: Return version, model, account info ...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ...us: Return version, model, account info - /rewind`: Handle conversation rewind ## Fronten...
(QB_NEW_EN)
[grammar] ~37-~37: There might be a mistake here.
Context: ....jsx) ### 3. Command Discovery on Mount - Fetch commands list on component mount/p...
(QB_NEW_EN)
[grammar] ~42-~42: There might be a mistake here.
Context: ...### 4. Input Handler for Slash Detection - Monitor textarea input changes - Detect ...
(QB_NEW_EN)
[grammar] ~50-~50: There might be a mistake here.
Context: ...lacement ### 5. Command Autocomplete UI - Render dropdown menu below cursor when `...
(QB_NEW_EN)
[grammar] ~51-~51: There might be a mistake here.
Context: ... Render dropdown menu below cursor when showCommandMenu === true - Display filtered commands with: - Comm...
(QB_NEW_EN)
[grammar] ~52-~52: There might be a mistake here.
Context: ... true` - Display filtered commands with: - Command name (bold) - Description/name...
(QB_NEW_EN)
[grammar] ~53-~53: There might be a mistake here.
Context: ...d commands with: - Command name (bold) - Description/namespace (gray text) - Ke...
(QB_NEW_EN)
[grammar] ~54-~54: There might be a mistake here.
Context: ...d) - Description/namespace (gray text) - Keyboard shortcut hint - Implement keybo...
(QB_NEW_EN)
[grammar] ~55-~55: There might be a mistake here.
Context: ...e (gray text) - Keyboard shortcut hint - Implement keyboard navigation: - Arrow...
(QB_NEW_EN)
[grammar] ~56-~56: There might be a mistake here.
Context: ...ut hint - Implement keyboard navigation: - ArrowUp/ArrowDown: Navigate commands -...
(QB_NEW_EN)
[grammar] ~57-~57: There might be a mistake here.
Context: ... - ArrowUp/ArrowDown: Navigate commands - Enter: Select command - Escape: Close ...
(QB_NEW_EN)
[grammar] ~58-~58: There might be a mistake here.
Context: ...igate commands - Enter: Select command - Escape: Close menu - Tab: Autocomplete...
(QB_NEW_EN)
[grammar] ~59-~59: There might be a mistake here.
Context: ...r: Select command - Escape: Close menu - Tab: Autocomplete first match - Implemen...
(QB_NEW_EN)
[grammar] ~60-~60: There might be a mistake here.
Context: ...e menu - Tab: Autocomplete first match - Implement mouse hover/click selection #...
(QB_NEW_EN)
[grammar] ~63-~63: There might be a mistake here.
Context: ...selection ### 6. Command Execution Flow - When command selected: - Extract comma...
(QB_NEW_EN)
[grammar] ~64-~64: There might be a mistake here.
Context: ... Execution Flow - When command selected: - Extract command name and arguments from ...
(QB_NEW_EN)
[grammar] ~65-~65: There might be a mistake here.
Context: ... - Extract command name and arguments from input - Call /api/commands/execute ...
(QB_NEW_EN)
[grammar] ~66-~66: There might be a mistake here.
Context: ... input - Call /api/commands/execute with command + args - For built-in command...
(QB_NEW_EN)
[grammar] ~67-~67: There might be a mistake here.
Context: ...-in commands that need special handling: - /clear: Clear chatMessages state - `/con...
(QB_NEW_EN)
[grammar] ~69-~69: There might be a mistake here.
Context: ...r chatMessages state - /config: Open settings modal - /memory: Open fi...
(QB_NEW_EN)
[grammar] ~70-~70: There might be a mistake here.
Context: ...: Open settings modal - /memory`: Open file editor for CLAUDE.md - For regul...
(QB_NEW_EN)
[grammar] ~71-~71: There might be a mistake here.
Context: ... for CLAUDE.md - For regular commands: - Get processed content from API - Inj...
(QB_NEW_EN)
[grammar] ~72-~72: There might be a mistake here.
Context: ...r commands: - Get processed content from API - Inject as system message/cont...
(QB_NEW_EN)
[grammar] ~74-~74: There might be a mistake here.
Context: ...em message/context - Send to Claude with modified prompt - Clear input and close...
(QB_NEW_EN)
[grammar] ~75-~75: There might be a mistake here.
Context: ... with modified prompt - Clear input and close menu after execution ### 7. Special Co...
(QB_NEW_EN)
[grammar] ~78-~78: There might be a mistake here.
Context: ...set chat state immediately (no API call) - /config: Toggle settings modal - /model: ...
(QB_NEW_EN)
[grammar] ~79-~79: There might be a mistake here.
Context: ...) - /config: Toggle settings modal - /model: Show model selection UI - /cost:...
(QB_NEW_EN)
[grammar] ~80-~80: There might be a mistake here.
Context: ... - /model: Show model selection UI - /cost: Display token usage overlay/modal - **...
(QB_NEW_EN)
[grammar] ~81-~81: There might be a mistake here.
Context: ...st**: Display token usage overlay/modal - **Commands with bash (!`)**: Show confirm...
(QB_NEW_EN)
[grammar] ~82-~82: There might be a mistake here.
Context: ...)**: Show confirmation before execution - **Commands with file refs (@`)**: Show fi...
(QB_NEW_EN)
[grammar] ~87-~87: There might be a mistake here.
Context: ...dditional Features ### 8. Command Hints - Show placeholder text: "Type / for comma...
(QB_NEW_EN)
[grammar] ~92-~92: There might be a mistake here.
Context: ...nd list ### 9. MCP Commands Integration - Query MCP servers for available prompts ...
(QB_NEW_EN)
[grammar] ~97-~97: There might be a mistake here.
Context: ...ion via MCP tool ### 10. Error Handling - Handle missing command files gracefully ...
(QB_NEW_EN)
[grammar] ~113-~113: There might be a mistake here.
Context: ...jsx` - Input handling, command execution - Add command execution to WebSocket messa...
(QB_NEW_EN)
[grammar] ~116-~116: There might be a mistake here.
Context: ...ocket message flow ## Testing Checklist - [ ] Built-in commands work (/help, /clea...
(QB_NEW_EN)
[grammar] ~121-~121: There might be a mistake here.
Context: ...2) - [ ] Autocomplete filters correctly as user types - [ ] Keyboard navigation wo...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.18.1)
ARCHITECTURE.md
31-31: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
ARCHITECTURE.md
Outdated
| - GET `/api/config` - Get WebSocket configuration | ||
| - GET `/api/browse-filesystem` - Browse filesystem for project selection | ||
| - POST `/api/transcribe` - Audio transcription | ||
| - GET `/api/sessions/:sessionId/token-usage` - Get token usage stats | ||
| - `/api/git/*` - Git operations (via git.js router) |
There was a problem hiding this comment.
Fix the documented token-usage endpoint.
GET /api/sessions/:sessionId/token-usage doesn’t exist; the code exposes /api/projects/:projectName/sessions/:sessionId/token-usage. Update the documentation so readers hit the working route.
🧰 Tools
🪛 LanguageTool
[grammar] ~82-~82: There might be a mistake here.
Context: ...pi/config- Get WebSocket configuration - GET/api/browse-filesystem` - Browse fi...
(QB_NEW_EN)
[grammar] ~83-~83: There might be a mistake here.
Context: ... Browse filesystem for project selection - POST /api/transcribe - Audio transcrip...
(QB_NEW_EN)
[grammar] ~84-~84: There might be a mistake here.
Context: ... /api/transcribe - Audio transcription - GET `/api/sessions/:sessionId/token-usag...
(QB_NEW_EN)
🤖 Prompt for AI Agents
In ARCHITECTURE.md around lines 82 to 86, the documented token-usage endpoint is
incorrect; update the entry from GET `/api/sessions/:sessionId/token-usage` to
the actual route GET
`/api/projects/:projectName/sessions/:sessionId/token-usage` so the docs match
the implementation and users can call the working endpoint.
| async function abortClaudeSDKSession(sessionId) { | ||
| const session = getSession(sessionId); | ||
|
|
||
| if (!session) { | ||
| console.log(`Session ${sessionId} not found`); | ||
| return false; | ||
| } | ||
|
|
||
| try { | ||
| console.log(`🛑 Aborting SDK session: ${sessionId}`); | ||
|
|
||
| // Call interrupt() on the query instance | ||
| await session.instance.interrupt(); | ||
|
|
||
| // Update session status | ||
| session.status = 'aborted'; | ||
|
|
||
| // Clean up temporary image files | ||
| await cleanupTempFiles(session.tempImagePaths, session.tempDir); | ||
|
|
||
| // Clean up session | ||
| removeSession(sessionId); | ||
|
|
||
| return true; | ||
| } catch (error) { | ||
| console.error(`Error aborting session ${sessionId}:`, error); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Session cleanup should run even if interrupt throws.
If session.instance.interrupt() rejects, we skip removing the session and cleaning up temp files, leaving the map polluted. Wrap the interrupt call in a try/catch but move the cleanup into a finally block so it always happens.
- try {
- console.log(`🛑 Aborting SDK session: ${sessionId}`);
-
- // Call interrupt() on the query instance
- await session.instance.interrupt();
-
- // Update session status
- session.status = 'aborted';
-
- // Clean up temporary image files
- await cleanupTempFiles(session.tempImagePaths, session.tempDir);
-
- // Clean up session
- removeSession(sessionId);
-
- return true;
- } catch (error) {
- console.error(`Error aborting session ${sessionId}:`, error);
- return false;
- }
+ console.log(`🛑 Aborting SDK session: ${sessionId}`);
+ let success = true;
+ try {
+ await session.instance.interrupt();
+ session.status = 'aborted';
+ } catch (error) {
+ console.error(`Error aborting session ${sessionId}:`, error);
+ success = false;
+ } finally {
+ await cleanupTempFiles(session.tempImagePaths, session.tempDir);
+ removeSession(sessionId);
+ }
+ return success;📝 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.
| async function abortClaudeSDKSession(sessionId) { | |
| const session = getSession(sessionId); | |
| if (!session) { | |
| console.log(`Session ${sessionId} not found`); | |
| return false; | |
| } | |
| try { | |
| console.log(`🛑 Aborting SDK session: ${sessionId}`); | |
| // Call interrupt() on the query instance | |
| await session.instance.interrupt(); | |
| // Update session status | |
| session.status = 'aborted'; | |
| // Clean up temporary image files | |
| await cleanupTempFiles(session.tempImagePaths, session.tempDir); | |
| // Clean up session | |
| removeSession(sessionId); | |
| return true; | |
| } catch (error) { | |
| console.error(`Error aborting session ${sessionId}:`, error); | |
| return false; | |
| } | |
| async function abortClaudeSDKSession(sessionId) { | |
| const session = getSession(sessionId); | |
| if (!session) { | |
| console.log(`Session ${sessionId} not found`); | |
| return false; | |
| } | |
| console.log(`🛑 Aborting SDK session: ${sessionId}`); | |
| let success = true; | |
| try { | |
| await session.instance.interrupt(); | |
| session.status = 'aborted'; | |
| } catch (error) { | |
| console.error(`Error aborting session ${sessionId}:`, error); | |
| success = false; | |
| } finally { | |
| await cleanupTempFiles(session.tempImagePaths, session.tempDir); | |
| removeSession(sessionId); | |
| } | |
| return success; | |
| } |
🤖 Prompt for AI Agents
In server/claude-sdk.js around lines 458 to 485, the call to
session.instance.interrupt() can reject and prevent session removal and temp
file cleanup; wrap the interrupt() call in its own try/catch to log any
interrupt error, then move the session cleanup (setting status to 'aborted',
await cleanupTempFiles(session.tempImagePaths, session.tempDir), and
removeSession(sessionId)) into a finally block so it always runs regardless of
interrupt outcome, and ensure cleanupTempFiles is called defensively (check for
undefined/empty paths) before awaiting.
| websocket.onmessage = (event) => { | ||
| try { | ||
| const data = JSON.parse(event.data); | ||
| console.log('🟢 WebSocket received:', data.type, data); |
There was a problem hiding this comment.
Remove or gate debug logging to prevent sensitive data exposure.
Logging the entire WebSocket message payload to the browser console risks exposing sensitive information such as PII (emails, user identifiers), authentication tokens, or internal API details. Browser console logs are accessible to users, browser extensions, and client-side scripts.
Consider one of these approaches:
- Preferred: Remove the log statement entirely for production, or gate it behind an environment check:
- console.log('🟢 WebSocket received:', data.type, data);
+ if (import.meta.env.DEV) {
+ console.log('🟢 WebSocket received:', data.type, data);
+ }- Alternative: Log only non-sensitive fields:
- console.log('🟢 WebSocket received:', data.type, data);
+ console.log('🟢 WebSocket received:', data.type);- If comprehensive logging is required: Implement a sanitization function to redact sensitive fields before logging.
📝 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.
| console.log('🟢 WebSocket received:', data.type, data); | |
| if (import.meta.env.DEV) { | |
| console.log('🟢 WebSocket received:', data.type, data); | |
| } |
🤖 Prompt for AI Agents
In src/utils/websocket.js around line 70, the console.log prints entire
WebSocket message payload which can expose sensitive data; remove this debug log
or gate it behind an environment flag (only log when NODE_ENV !== 'production'
or a DEBUG flag is set), or replace it with logging of a minimal safe subset
(e.g., message type and non-sensitive metadata) and/or pass the payload through
a sanitization/redaction function before logging.
- Add responsive positioning logic that detects mobile screens (< 640px) - On mobile: Position menu from bottom (80px above input) with full width - On desktop: Use calculated top position with boundary checks - Ensure menu stays within viewport on all screen sizes - Use Math.max/min to prevent menu from going off-screen - Apply consistent positioning to both empty and populated menu states Technical changes: - Add getMenuPosition() function to calculate responsive styles - Mobile: bottom-anchored, full-width with 16px margins - Desktop: top-anchored with viewport boundary constraints - Spread menuPosition styles into both menu render cases 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/ChatInterface.jsx (3)
2388-2615: Wrap case 'claude-response' in a block (noSwitchDeclarations).The
messageDataconstant at line 2389 is declared at case scope and can be accessed by other cases, violatingnoSwitchDeclarations. This matches the static analysis warning.Apply this diff to wrap the case body in a block:
- case 'claude-response': - const messageData = latestMessage.data.message || latestMessage.data; + case 'claude-response': { + const messageData = latestMessage.data.message || latestMessage.data; // ... rest of the case logic ... break; + }
2672-2736: Wrap case 'cursor-result' declarations in a block (noSwitchDeclarations).The constants
cursorCompletedSessionId(line 2674) andpendingCursorSessionId(line 2726) are declared at case scope and can be accessed by other cases. This matches the static analysis warnings.Apply this diff to wrap the case body in a block:
- case 'cursor-result': - // Get session ID from message or fall back to current session - const cursorCompletedSessionId = latestMessage.sessionId || currentSessionId; + case 'cursor-result': { + const cursorCompletedSessionId = latestMessage.sessionId || currentSessionId; // ... rest of the case logic including pendingCursorSessionId ... break; + }
2769-2823: Wrap case 'claude-complete' declarations in a block (noSwitchDeclarations).The constant
completedSessionId(line 2771) is declared at case scope and can be accessed by other cases. This matches the static analysis warning.Apply this diff to wrap the case body in a block:
- case 'claude-complete': - // Get session ID from message or fall back to current session - const completedSessionId = latestMessage.sessionId || currentSessionId || sessionStorage.getItem('pendingSessionId'); + case 'claude-complete': { + const completedSessionId = latestMessage.sessionId || currentSessionId || sessionStorage.getItem('pendingSessionId'); // ... rest of the case logic ... break; + }
♻️ Duplicate comments (1)
src/components/ChatInterface.jsx (1)
1219-1219: Add unmount cleanup for commandQueryTimerRef.As flagged in a previous review, the
commandQueryTimerRefdebounce timer lacks cleanup on component unmount. MultipleclearTimeoutcalls exist during normal operation (lines 3372-3374, 3421-3423, 3578-3582, 3590-3592), but no unmount effect was added.Add a cleanup effect:
useEffect(() => { return () => { if (commandQueryTimerRef.current) { clearTimeout(commandQueryTimerRef.current); } }; }, []);Place this near line 1219 where the ref is declared, or with other cleanup effects.
🧹 Nitpick comments (3)
src/components/CommandMenu.jsx (2)
18-44: Consider adding window resize listener for responsive positioning.The
getMenuPosition()function is called once per render, but window size changes (e.g., device rotation, browser resize) won't trigger a recalculation until the next render. If the menu is open during a resize, positioning could be incorrect.Add a window resize listener to update positioning dynamically:
+ const [windowSize, setWindowSize] = useState({ width: window.innerWidth, height: window.innerHeight }); + + useEffect(() => { + const handleResize = () => { + setWindowSize({ width: window.innerWidth, height: window.innerHeight }); + }; + window.addEventListener('resize', handleResize); + return () => window.removeEventListener('resize', handleResize); + }, []); + const getMenuPosition = () => { - const isMobile = window.innerWidth < 640; - const viewportHeight = window.innerHeight; + const isMobile = windowSize.width < 640; + const viewportHeight = windowSize.height;
173-175: Optimize command lookup during rendering.The
find()call on line 174 runs for every command in every group during each render, resulting in O(n²) complexity for the entire list. With 57 commands mentioned in the PR, this could cause rendering lag.Build a Map for O(1) lookups:
// Calculate global index for each command let globalIndex = 0; const commandsWithIndex = []; + const commandIndexMap = new Map(); orderedNamespaces.forEach(namespace => { groupedCommands[namespace].forEach(command => { - commandsWithIndex.push({ + const cmdWithIndex = { ...command, globalIndex: globalIndex++, namespace - }); + }; + commandsWithIndex.push(cmdWithIndex); + commandIndexMap.set(`${namespace}-${command.name}`, cmdWithIndex); }); });Then replace the
find()call:{groupedCommands[namespace].map((command) => { - const cmdWithIndex = commandsWithIndex.find(c => c.name === command.name && c.namespace === namespace); + const cmdWithIndex = commandIndexMap.get(`${namespace}-${command.name}`); const isSelected = cmdWithIndex && cmdWithIndex.globalIndex === selectedIndex;src/components/ChatInterface.jsx (1)
3546-3560: Consider more robust code block detection.The code block detection on lines 3551-3552 counts triple backticks to determine if the cursor is inside a code block. This simple approach has limitations:
- Doesn't account for inline code (single backticks)
- Could be fooled by escaped backticks
- May not work correctly with nested or malformed markdown
For a more robust solution, consider parsing the markdown structure or using a markdown parser library. However, the current implementation is acceptable for common use cases.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/ChatInterface.jsx(30 hunks)src/components/CommandMenu.jsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/components/ChatInterface.jsx (3)
src/App.jsx (6)
showThinking(60-60)selectedProject(49-49)selectedSession(50-50)processingSessions(71-71)isInputFocused(55-55)sendByCtrlEnter(62-62)src/utils/api.js (3)
authenticatedFetch(2-20)authenticatedFetch(2-20)url(53-53)src/components/CommandMenu.jsx (1)
CommandMenu(13-313)
src/components/CommandMenu.jsx (1)
src/components/ChatInterface.jsx (1)
frequentCommands(1371-1397)
🪛 Biome (2.1.2)
src/components/ChatInterface.jsx
[error] 2389-2391: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2674-2676: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2726-2727: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2771-2773: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (3)
src/components/ChatInterface.jsx (3)
3071-3101: Token usage fetch implementation looks good.The refactored token budget fetching now correctly:
- Fetches once on session load (no polling)
- Resets for new/empty sessions
- Uses
authenticatedFetchwith proper error handling- Fetches updated usage after message completion (lines 2779-2794)
This resolves the AbortController pattern issue from the previous review.
3350-3387: Command insertion logic correctly implemented.The
selectCommandfunction now:
- Inserts
command.namedirectly without prepending '/' (line 3359) - fixing the double-slash issue- Does not call
handleCommandSelect- fixing the premature execution issue- Properly manages cursor position and focus
Both issues flagged in previous reviews have been resolved.
1586-1617: Bash command confirmation provides good security.The
handleCustomCommandfunction correctly shows a confirmation dialog for commands containing bash execution (lines 1591-1603). This prevents accidental execution of potentially destructive commands and follows security best practices.
- Add useEffect hook to detect clicks outside the menu and close it - Fix mobile positioning to use calculated position from textarea instead of hardcoded bottom value - Ensure menu appears just above the input on mobile with proper spacing - Keep full-width layout on mobile screens (< 640px) - Maintain viewport boundary checks on both mobile and desktop Technical changes: - Add mousedown event listener to document when menu is open - Check if click target is outside menuRef and call onClose - Remove hardcoded `bottom: '80px'` in favor of calculated `top` position - Use Math.max to ensure menu stays at least 16px from top edge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/components/CommandMenu.jsx (3)
17-48: Consider memoizinggetMenuPositionand extracting magic numbers.The
getMenuPositionfunction is recalculated on every render. Since it depends on thepositionprop, consider memoizing it withuseMemoto avoid unnecessary recalculations. Additionally, the menu height values (308, 316) are hardcoded magic numbers that should be extracted as named constants for better maintainability.Apply this diff to memoize the calculation and extract constants:
+// Menu dimensions +const MENU_HEIGHT = 308; +const MENU_HEIGHT_WITH_PADDING = 316; +const VIEWPORT_PADDING = 16; + const CommandMenu = ({ commands = [], selectedIndex = -1, onSelect, onClose, position = { top: 0, left: 0 }, isOpen = false, frequentCommands = [] }) => { const menuRef = useRef(null); const selectedItemRef = useRef(null); - // Calculate responsive positioning - const getMenuPosition = () => { + const menuPosition = useMemo(() => { const isMobile = window.innerWidth < 640; const viewportHeight = window.innerHeight; - const menuHeight = 308; // Approximate menu height if (isMobile) { // On mobile, position just above the input by calculating from top // Ensure it doesn't go above the viewport - const calculatedTop = Math.max(16, position.top); + const calculatedTop = Math.max(VIEWPORT_PADDING, position.top); return { position: 'fixed', top: `${calculatedTop}px`, - left: '16px', - right: '16px', + left: `${VIEWPORT_PADDING}px`, + right: `${VIEWPORT_PADDING}px`, width: 'auto', - maxWidth: 'calc(100vw - 32px)' + maxWidth: `calc(100vw - ${VIEWPORT_PADDING * 2}px)` }; } // On desktop, use provided position but ensure it stays on screen return { position: 'fixed', - top: `${Math.max(16, Math.min(position.top, viewportHeight - 316))}px`, + top: `${Math.max(VIEWPORT_PADDING, Math.min(position.top, viewportHeight - MENU_HEIGHT_WITH_PADDING))}px`, left: `${position.left}px`, - width: 'min(400px, calc(100vw - 32px))', - maxWidth: 'calc(100vw - 32px)' + width: `min(400px, calc(100vw - ${VIEWPORT_PADDING * 2}px))`, + maxWidth: `calc(100vw - ${VIEWPORT_PADDING * 2}px)` }; - }; - - const menuPosition = getMenuPosition(); + }, [position]);
306-330: Consider alternatives to!importantin dark mode styles.The dark mode implementation uses
!importantextensively to override inline styles. While this works, it makes the styles harder to customize or override if needed. For future improvements, consider using CSS modules, styled-components, or CSS variables to avoid the need for!important.
156-175: Consider addingaria-activedescendantfor improved screen reader support.The component has good ARIA attributes, but adding
aria-activedescendantto the listbox would improve screen reader navigation by explicitly indicating which option is currently active.Example implementation:
+ // Generate a unique ID for the selected item + const selectedItemId = selectedIndex >= 0 ? `command-item-${selectedIndex}` : undefined; + <div ref={menuRef} role="listbox" aria-label="Available commands" + aria-activedescendant={selectedItemId} className="command-menu"And then add the
idattribute to each command item:<div - key={`${namespace}-${command.name}`} + key={`${namespace}-${command.name}`} + id={isSelected ? selectedItemId : undefined} ref={isSelected ? selectedItemRef : null}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/CommandMenu.jsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/CommandMenu.jsx (1)
src/components/ChatInterface.jsx (1)
frequentCommands(1371-1397)
🔇 Additional comments (2)
src/components/CommandMenu.jsx (2)
204-205: Well-implemented interaction handlers.The event handlers correctly distinguish between provisional selection (mouse hover) and final selection (click), and the
preventDefaultononMouseDownproperly prevents the textarea from losing focus.Also applies to: 216-216
1-335: Well-structured component with good UX considerations.The
CommandMenucomponent is well-implemented with proper responsive positioning, keyboard navigation support, dark mode, and accessibility features. The grouping logic and visual hierarchy make it easy for users to find commands. The handling of frequent commands integration is particularly nice.
| {/* Command icon based on namespace */} | ||
| <span | ||
| style={{ | ||
| fontSize: '16px', | ||
| flexShrink: 0 | ||
| }} | ||
| > | ||
| {namespace === 'builtin' && '⚡'} | ||
| {namespace === 'project' && '📁'} | ||
| {namespace === 'user' && '👤'} | ||
| {namespace === 'other' && '📝'} | ||
| </span> |
There was a problem hiding this comment.
Add icon for 'frequent' namespace.
The code adds a 'frequent' namespace at line 126, but the icon rendering logic doesn't handle it. Frequent commands will appear without an icon.
Apply this diff to add the frequent icon:
<span
style={{
fontSize: '16px',
flexShrink: 0
}}
>
+ {namespace === 'frequent' && '⭐'}
{namespace === 'builtin' && '⚡'}
{namespace === 'project' && '📁'}
{namespace === 'user' && '👤'}
{namespace === 'other' && '📝'}
</span>📝 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.
| {/* Command icon based on namespace */} | |
| <span | |
| style={{ | |
| fontSize: '16px', | |
| flexShrink: 0 | |
| }} | |
| > | |
| {namespace === 'builtin' && '⚡'} | |
| {namespace === 'project' && '📁'} | |
| {namespace === 'user' && '👤'} | |
| {namespace === 'other' && '📝'} | |
| </span> | |
| {/* Command icon based on namespace */} | |
| <span | |
| style={{ | |
| fontSize: '16px', | |
| flexShrink: 0 | |
| }} | |
| > | |
| {namespace === 'frequent' && '⭐'} | |
| {namespace === 'builtin' && '⚡'} | |
| {namespace === 'project' && '📁'} | |
| {namespace === 'user' && '👤'} | |
| {namespace === 'other' && '📝'} | |
| </span> |
🤖 Prompt for AI Agents
In src/components/CommandMenu.jsx around lines 227 to 238, the namespace icon
span does not handle the newly added 'frequent' namespace so frequent commands
show no icon; add a branch to render an icon for namespace === 'frequent' (e.g.,
'⭐' or another chosen emoji) alongside the existing checks, keeping the same
style and ordering so frequent commands display the icon consistently.
- Add console logs to debug positioning and rendering - Improve mobile positioning with better space calculations - Check if there's enough space above textarea before positioning - Position from top of viewport if insufficient space above input - Ensure menu stays within visible viewport boundaries Debugging additions: - Log isOpen, commandsLength, position, and menuPosition - Log mobile positioning calculations 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/components/CommandMenu.jsx (1)
239-250: Add icon for 'frequent' namespace.The code adds a 'frequent' namespace at line 138, but the icon rendering logic doesn't handle it. Frequent commands will appear without an icon.
Apply this diff to add the frequent icon:
<span style={{ fontSize: '16px', flexShrink: 0 }} > + {namespace === 'frequent' && '⭐'} {namespace === 'builtin' && '⚡'} {namespace === 'project' && '📁'} {namespace === 'user' && '👤'} {namespace === 'other' && '📝'} </span>
🧹 Nitpick comments (2)
src/components/CommandMenu.jsx (2)
156-166: Consider pre-attaching globalIndex to avoid repeated lookups.The current implementation performs a
findoperation (line 206) for each command during rendering. While not a bottleneck at current scale (57 commands per PR notes), you could eliminate this overhead by attachingglobalIndexdirectly to the grouped commands.Apply this diff to optimize the lookup:
// Calculate global index for each command let globalIndex = 0; - const commandsWithIndex = []; orderedNamespaces.forEach(namespace => { groupedCommands[namespace].forEach(command => { - commandsWithIndex.push({ - ...command, - globalIndex: globalIndex++, - namespace - }); + command.globalIndex = globalIndex++; + command.namespace = namespace; }); });Then simplify line 206-207:
- const cmdWithIndex = commandsWithIndex.find(c => c.name === command.name && c.namespace === namespace); - const isSelected = cmdWithIndex && cmdWithIndex.globalIndex === selectedIndex; + const isSelected = command.globalIndex === selectedIndex;And line 216:
- onMouseEnter={() => onSelect && onSelect(command, cmdWithIndex.globalIndex, true)} + onMouseEnter={() => onSelect && onSelect(command, command.globalIndex, true)}And line 217:
- onClick={() => onSelect && onSelect(command, cmdWithIndex.globalIndex, false)} + onClick={() => onSelect && onSelect(command, command.globalIndex, false)}Also applies to: 205-207
318-342: Consider externalizing dark mode styles.The inline
<style>tag with!importantoverrides works but mixes inline and CSS-based styling. For better maintainability, consider moving these styles to a separate CSS/SCSS module or using a CSS-in-JS solution.Example approach with a separate CSS module:
/* CommandMenu.module.css */ @media (prefers-color-scheme: dark) { .commandMenu { background-color: #1f2937; border-color: #374151; } .commandItem[aria-selected="true"] { background-color: #1e40af; } .commandItem span:not(.commandMetadataBadge) { color: #f3f4f6; } .commandMetadataBadge { background-color: #f3f4f6; color: #6b7280; } .commandItem div { color: #d1d5db; } .commandGroup > div:first-child { color: #9ca3af; } }Then import and apply class names instead of inline styles.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/CommandMenu.jsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/CommandMenu.jsx (1)
src/components/ChatInterface.jsx (1)
frequentCommands(1371-1397)
| /** | ||
| * CommandMenu - Autocomplete dropdown for slash commands | ||
| * | ||
| * @param {Array} commands - Array of command objects to display | ||
| * @param {number} selectedIndex - Currently selected command index | ||
| * @param {Function} onSelect - Callback when a command is selected | ||
| * @param {Function} onClose - Callback when menu should close | ||
| * @param {Object} position - Position object { top, left } for absolute positioning | ||
| * @param {boolean} isOpen - Whether the menu is open | ||
| */ | ||
| const CommandMenu = ({ commands = [], selectedIndex = -1, onSelect, onClose, position = { top: 0, left: 0 }, isOpen = false, frequentCommands = [] }) => { |
There was a problem hiding this comment.
Document the frequentCommands prop in JSDoc.
The frequentCommands prop is accepted by the component but not documented in the JSDoc comment block.
Apply this diff to add the missing parameter:
* @param {Function} onClose - Callback when menu should close
* @param {Object} position - Position object { top, left } for absolute positioning
* @param {boolean} isOpen - Whether the menu is open
+ * @param {Array} frequentCommands - Array of frequently used command objects
*/📝 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.
| /** | |
| * CommandMenu - Autocomplete dropdown for slash commands | |
| * | |
| * @param {Array} commands - Array of command objects to display | |
| * @param {number} selectedIndex - Currently selected command index | |
| * @param {Function} onSelect - Callback when a command is selected | |
| * @param {Function} onClose - Callback when menu should close | |
| * @param {Object} position - Position object { top, left } for absolute positioning | |
| * @param {boolean} isOpen - Whether the menu is open | |
| */ | |
| const CommandMenu = ({ commands = [], selectedIndex = -1, onSelect, onClose, position = { top: 0, left: 0 }, isOpen = false, frequentCommands = [] }) => { | |
| /** | |
| * CommandMenu - Autocomplete dropdown for slash commands | |
| * | |
| * @param {Array} commands - Array of command objects to display | |
| * @param {number} selectedIndex - Currently selected command index | |
| * @param {Function} onSelect - Callback when a command is selected | |
| * @param {Function} onClose - Callback when menu should close | |
| * @param {Object} position - Position object { top, left } for absolute positioning | |
| * @param {boolean} isOpen - Whether the menu is open | |
| * @param {Array} frequentCommands - Array of frequently used command objects | |
| */ | |
| const CommandMenu = ({ commands = [], selectedIndex = -1, onSelect, onClose, position = { top: 0, left: 0 }, isOpen = false, frequentCommands = [] }) => { |
🤖 Prompt for AI Agents
In src/components/CommandMenu.jsx around lines 3 to 13, the JSDoc block is
missing documentation for the frequentCommands prop; add a new @param entry such
as "@param {Array} frequentCommands - Array of frequently used command objects
(used to highlight or prioritize in the menu)" and, if desired, note the default
value to match the function signature (default empty array) so the JSDoc
accurately reflects the component API.
- Change mobile positioning from top-based to bottom-based (90px from bottom) - This ensures menu always appears just above input, regardless of keyboard state - Add maxHeight: '50vh' to prevent menu from taking up too much space - Remove complex position calculations that didn't work well with mobile keyboard - Remove debug console.log statements - Menu now correctly appears above input on all mobile screen sizes Technical changes: - Mobile: Use fixed bottom positioning instead of calculated top - Desktop: Continue using top positioning for consistency - Simplified positioning logic for better maintainability 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/CommandMenu.jsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/CommandMenu.jsx (1)
src/components/ChatInterface.jsx (1)
frequentCommands(1371-1397)
🔇 Additional comments (1)
src/components/CommandMenu.jsx (1)
18-20: Verify SSR compatibility with window access.The
getMenuPositionfunction accesseswindow.innerWidthandwindow.innerHeightwithout checking ifwindowis defined. While the component returnsnullwhenisOpenisfalse, the positioning calculation on line 47 executes before that check. If this component is ever server-rendered whileisOpen={true}, it will throw aReferenceError.If your application uses server-side rendering, ensure this component is only rendered client-side or add a window check:
const getMenuPosition = () => { + if (typeof window === 'undefined') { + return { position: 'fixed', top: '0px', left: '0px', width: '400px' }; + } const isMobile = window.innerWidth < 640;
| if (commands.length === 0) { | ||
| return ( | ||
| <div | ||
| ref={menuRef} | ||
| className="command-menu" | ||
| style={{ | ||
| ...menuPosition, | ||
| maxHeight: '300px', | ||
| backgroundColor: 'white', | ||
| border: '1px solid #e5e7eb', | ||
| borderRadius: '8px', | ||
| boxShadow: '0 10px 15px -3px rgba(0, 0, 0, 0.1), 0 4px 6px -2px rgba(0, 0, 0, 0.05)', | ||
| zIndex: 1000, | ||
| padding: '20px', | ||
| opacity: 1, | ||
| transform: 'translateY(0)', | ||
| transition: 'opacity 150ms ease-in-out, transform 150ms ease-in-out', | ||
| textAlign: 'center', | ||
| color: '#6b7280' | ||
| }} | ||
| > | ||
| No commands available | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Add dark mode styling for empty state.
The empty state message has hardcoded light mode colors (backgroundColor: 'white', color: '#6b7280') but lacks dark mode overrides in the @media (prefers-color-scheme: dark) block (lines 306-329). In dark mode, this will display a white box with light gray text, resulting in poor contrast and visual inconsistency with the rest of the menu.
Add the following to the dark mode styles block to fix the empty state appearance:
.command-group > div:first-child {
color: #9ca3af !important;
}
+ .command-menu:has(div:only-child) {
+ background-color: #1f2937 !important;
+ border-color: #374151 !important;
+ color: #d1d5db !important;
+ }
}Alternatively, apply a class to the empty state div and target it specifically in the dark mode styles.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/components/CommandMenu.jsx around lines 84-108, the empty-state div uses
hardcoded light-mode inline styles (backgroundColor: 'white', color: '#6b7280')
and needs dark-mode overrides; update the component to either apply a specific
className (e.g., "command-menu__empty") or conditionally set inline styles based
on prefers-color-scheme, and then add dark-mode rules in your CSS/@media
(prefers-color-scheme: dark) to set a dark background (e.g., #0f172a or theme
token), lighter text (e.g., #9ca3af or similar), a darker border (e.g., #1f2937)
and adjusted box-shadow, so the empty state matches the menu's dark appearance
and maintains proper contrast.
| .command-item span:not(.command-metadata-badge) { | ||
| color: #f3f4f6 !important; | ||
| } |
There was a problem hiding this comment.
Selection indicator loses its blue color in dark mode.
The dark mode selector .command-item span:not(.command-metadata-badge) uses !important and matches all spans including the selection indicator "↵" (lines 288-297), which has an inline color: '#3b82f6'. This overrides the blue color with light gray #f3f4f6, making the selection indicator less visually distinct.
Refine the selector to exclude the selection indicator:
- .command-item span:not(.command-metadata-badge) {
+ .command-item > div > div > span:nth-child(1),
+ .command-item > div > div > span:nth-child(2) {
color: #f3f4f6 !important;
}Or add a class to the selection indicator span and exclude it from the selector:
<span
+ className="selection-indicator"
style={{
marginLeft: '8px',
color: '#3b82f6',- .command-item span:not(.command-metadata-badge) {
+ .command-item span:not(.command-metadata-badge):not(.selection-indicator) {
color: #f3f4f6 !important;
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/components/CommandMenu.jsx around lines 315-317, the dark-mode CSS rule
.command-item span:not(.command-metadata-badge) { color: #f3f4f6 !important; }
is too broad and overrides the selection indicator's inline blue color; refine
the rule to exclude the selection-indicator (e.g. .command-item
span:not(.command-metadata-badge):not(.selection-indicator)) or add a specific
class to the selection indicator span and exclude that class from the selector,
and remove the unnecessary !important so the indicator's blue color can apply.
API error messages were appearing as session titles because they come from assistant messages with isApiErrorMessage flag, but the filter only checked user messages. Updated assistant message handling to: - Skip messages with isApiErrorMessage: true flag - Filter messages starting with "Invalid API key" Also improved session title logic to prefer last user message over last assistant message for better context. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/projects.js (1)
652-762: messageCount increments on non-message entries; count only user/assistant messagesCurrently session.messageCount++ runs for every entry with sessionId (including summaries and system-ish entries). This inflates the count displayed in UI.
Restrict increments to actual chat messages:
- session.messageCount++; + if (entry.message?.role === 'user' || entry.message?.role === 'assistant') { + session.messageCount++; + }Everything else in the new summary/last-message tracking looks good.
♻️ Duplicate comments (5)
src/components/CommandMenu.jsx (4)
3-12: Document frequentCommands and mobile position.bottom in JSDocAdd missing params to reflect the actual API:
* @param {Object} position - Position object { top, left } for absolute positioning * @param {boolean} isOpen - Whether the menu is open + * @param {Array} frequentCommands - Frequently used commands to surface as a group + * @param {number} [position.bottom] - Optional bottom offset for mobile positioning
229-239: Add icon for 'frequent' namespaceFrequent group items render without an icon. Add a branch:
- {namespace === 'builtin' && '⚡'} + {namespace === 'frequent' && '⭐'} + {namespace === 'builtin' && '⚡'}
307-331: Dark mode CSS overrides the blue selection indicatorThe selector
.command-item span:not(.command-metadata-badge)overrides the "↵" indicator’s blue color. Exclude it or add a class:Option A (exclude indicator by class):
- Add className="selection-indicator" to the indicator span.
- Update CSS:
- .command-item span:not(.command-metadata-badge) { + .command-item span:not(.command-metadata-badge):not(.selection-indicator) { color: #f3f4f6 !important; }
85-110: Empty state lacks dark-mode stylingThe inline white bg and gray text don’t adapt to dark mode. Add a dark override in the existing style block or switch to CSS classes and theme tokens to match the menu’s dark palette.
server/index.js (1)
377-394: Fix path traversal / symlink escape checks.Using
startsWithonpath.resolve(filePath)doesn't prevent escapes:/home/user/project-evil/foopasses the prefix test, and a symlink inside the project can still point outside. Prior review called this out and it's still exploitable—an attacker can read or write arbitrary files. Please realpath both the project root and candidate path (or the parent directory for creates), then gate access viapath.relative(...).startsWith('..')checks so both prefix collisions and symlinks are blocked, and update the binary/save handlers the same way.- const resolved = path.resolve(filePath); - const normalizedRoot = path.resolve(projectRoot) + path.sep; - if (!resolved.startsWith(normalizedRoot)) { - return res.status(403).json({ error: 'Path must be under project root' }); - } + const projectRootReal = await fsPromises.realpath(projectRoot); + const resolved = path.resolve(filePath); + const relative = path.relative(projectRootReal, resolved); + if (relative.startsWith('..') || path.isAbsolute(relative)) { + return res.status(403).json({ error: 'Path must be under project root' }); + } + let resolvedReal; + try { + resolvedReal = await fsPromises.realpath(resolved); + } catch (error) { + if (error.code === 'ENOENT') { + return res.status(404).json({ error: 'File not found' }); + } + throw error; + } + const realRelative = path.relative(projectRootReal, resolvedReal); + if (realRelative.startsWith('..') || path.isAbsolute(realRelative)) { + return res.status(403).json({ error: 'Path must be under project root' }); + }Apply the same pattern (using the parent directory’s
realpathfor writes) for the binary download and save handlers to close the hole.Also applies to: 415-429, 483-487
🧹 Nitpick comments (2)
src/components/CommandMenu.jsx (2)
51-66: Support touch devices for outside-click closeUse pointerdown (or add touchstart) to catch taps on mobile:
- document.addEventListener('mousedown', handleClickOutside); + document.addEventListener('pointerdown', handleClickOutside); ... - document.removeEventListener('mousedown', handleClickOutside); + document.removeEventListener('pointerdown', handleClickOutside);
21-22: Remove or use menuHeight constantmenuHeight is defined but unused; either remove it or use it to cap desktop positioning. Nit, but keeps code tidy.
Also applies to: 39-47
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
server/index.js(14 hunks)server/projects.js(2 hunks)src/App.jsx(6 hunks)src/components/ChatInterface.jsx(36 hunks)src/components/CommandMenu.jsx(1 hunks)src/components/MainContent.jsx(2 hunks)src/components/Sidebar.jsx(1 hunks)src/components/TokenUsagePie.jsx(1 hunks)src/index.css(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/TokenUsagePie.jsx
- src/components/MainContent.jsx
🧰 Additional context used
🧬 Code graph analysis (5)
src/components/Sidebar.jsx (3)
src/components/TaskIndicator.jsx (1)
TaskIndicator(11-106)src/components/TaskMasterStatus.jsx (2)
projectConfigured(37-37)mcpConfigured(34-34)src/contexts/TaskMasterContext.jsx (1)
mcpServerStatus(54-54)
server/projects.js (1)
server/index.js (3)
fileStream(443-443)filePath(411-411)entry(1178-1178)
src/components/CommandMenu.jsx (1)
src/components/ChatInterface.jsx (1)
frequentCommands(1384-1410)
src/components/ChatInterface.jsx (3)
src/App.jsx (7)
showThinking(60-60)selectedSession(50-50)selectedProject(49-49)externalMessageUpdate(75-75)processingSessions(71-71)isInputFocused(55-55)sendByCtrlEnter(62-62)src/utils/api.js (3)
authenticatedFetch(2-20)authenticatedFetch(2-20)url(53-53)src/components/CommandMenu.jsx (1)
CommandMenu(13-334)
server/index.js (4)
server/projects.js (8)
projectName(962-962)projectDir(271-271)projectDir(525-525)projectDir(776-776)projectDir(863-863)projectDir(926-926)projectDir(966-966)lines(877-877)server/routes/commands.js (2)
sessionId(221-221)homeDir(394-394)server/claude-sdk.js (7)
mimeType(215-215)activeSessions(21-21)contextWindow(174-174)inputTokens(164-164)cacheCreationTokens(167-167)cacheReadTokens(166-166)totalUsed(170-170)server/cursor-cli.js (1)
process(239-239)
🪛 Biome (2.1.2)
src/components/ChatInterface.jsx
[error] 2461-2463: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2746-2749: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2798-2799: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2843-2846: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (2)
src/index.css (1)
723-733: LGTM: block-level placeholder + height auto; solid dark placeholder overrides
- display: block on .chat-input-placeholder and :not(:focus) height auto address shrinking/flow issues.
- Dark-mode placeholder overrides scoped to .chat-input-placeholder avoid over-bright text.
Please verify inputs using .chat-input-placeholder (textarea vs input) don’t regress layout due to block display in narrow containers.
Also applies to: 447-462
src/App.jsx (1)
715-727: Processing state wiring and “Show thinking” propagation look goodProps onSessionProcessing/onSessionNotProcessing/processingSessions and showThinking are correctly threaded to MainContent and QuickSettingsPanel.
Also applies to: 747-749
| // External Session Update Detection: Check if the changed file is the current session's JSONL | ||
| // If so, and the session is not active, trigger a message reload in ChatInterface | ||
| if (latestMessage.changedFile && selectedSession && selectedProject) { | ||
| // Extract session ID from changedFile (format: "project-name/session-id.jsonl") | ||
| const changedFileParts = latestMessage.changedFile.split('/'); | ||
| if (changedFileParts.length >= 2) { | ||
| const filename = changedFileParts[changedFileParts.length - 1]; | ||
| const changedSessionId = filename.replace('.jsonl', ''); | ||
|
|
||
| // Check if this is the currently-selected session | ||
| if (changedSessionId === selectedSession.id) { | ||
| const isSessionActive = activeSessions.has(selectedSession.id); | ||
|
|
||
| if (!isSessionActive) { | ||
| // Session is not active - safe to reload messages | ||
| console.log('🔄 External CLI update detected for current session:', changedSessionId); | ||
| setExternalMessageUpdate(prev => prev + 1); | ||
| } else { | ||
| // Session is active - skip reload to avoid interrupting user | ||
| console.log('⏸️ External update paused - session is active:', changedSessionId); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Windows path compatibility: split changedFile on both separators
changedFile splitting uses '/', which fails on Windows paths (e.g., C:...jsonl). Use a cross‑platform split:
- const changedFileParts = latestMessage.changedFile.split('/');
+ const changedFileParts = latestMessage.changedFile.split(/[\\/]/);This ensures correct sessionId extraction on all platforms.
📝 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.
| // External Session Update Detection: Check if the changed file is the current session's JSONL | |
| // If so, and the session is not active, trigger a message reload in ChatInterface | |
| if (latestMessage.changedFile && selectedSession && selectedProject) { | |
| // Extract session ID from changedFile (format: "project-name/session-id.jsonl") | |
| const changedFileParts = latestMessage.changedFile.split('/'); | |
| if (changedFileParts.length >= 2) { | |
| const filename = changedFileParts[changedFileParts.length - 1]; | |
| const changedSessionId = filename.replace('.jsonl', ''); | |
| // Check if this is the currently-selected session | |
| if (changedSessionId === selectedSession.id) { | |
| const isSessionActive = activeSessions.has(selectedSession.id); | |
| if (!isSessionActive) { | |
| // Session is not active - safe to reload messages | |
| console.log('🔄 External CLI update detected for current session:', changedSessionId); | |
| setExternalMessageUpdate(prev => prev + 1); | |
| } else { | |
| // Session is active - skip reload to avoid interrupting user | |
| console.log('⏸️ External update paused - session is active:', changedSessionId); | |
| } | |
| } | |
| } | |
| } | |
| // External Session Update Detection: Check if the changed file is the current session's JSONL | |
| // If so, and the session is not active, trigger a message reload in ChatInterface | |
| if (latestMessage.changedFile && selectedSession && selectedProject) { | |
| // Extract session ID from changedFile (format: "project-name/session-id.jsonl") | |
| const changedFileParts = latestMessage.changedFile.split(/[\\/]/); | |
| if (changedFileParts.length >= 2) { | |
| const filename = changedFileParts[changedFileParts.length - 1]; | |
| const changedSessionId = filename.replace('.jsonl', ''); | |
| // Check if this is the currently-selected session | |
| if (changedSessionId === selectedSession.id) { | |
| const isSessionActive = activeSessions.has(selectedSession.id); | |
| if (!isSessionActive) { | |
| // Session is not active - safe to reload messages | |
| console.log('🔄 External CLI update detected for current session:', changedSessionId); | |
| setExternalMessageUpdate(prev => prev + 1); | |
| } else { | |
| // Session is active - skip reload to avoid interrupting user | |
| console.log('⏸️ External update paused - session is active:', changedSessionId); | |
| } | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/App.jsx around lines 172 to 196, changedFile is split only on '/' which
breaks for Windows backslashes; update the extraction to be cross-platform by
normalizing the path or splitting on both separators (e.g., replace backslashes
with forward slashes or split with a regex like /[\/\\]/) or use a path-basename
utility to get the filename, then strip the '.jsonl' suffix as before so
changedSessionId is correctly extracted on all platforms.
| if (showCommandMenu && filteredCommands.length > 0) { | ||
| if (e.key === 'ArrowDown') { | ||
| e.preventDefault(); | ||
| setSelectedCommandIndex(prev => | ||
| prev < filteredCommands.length - 1 ? prev + 1 : 0 | ||
| ); | ||
| return; | ||
| } | ||
| if (e.key === 'ArrowUp') { | ||
| e.preventDefault(); | ||
| setSelectedCommandIndex(prev => | ||
| prev > 0 ? prev - 1 : filteredCommands.length - 1 | ||
| ); | ||
| return; | ||
| } | ||
| if (e.key === 'Tab' || e.key === 'Enter') { | ||
| e.preventDefault(); | ||
| if (selectedCommandIndex >= 0) { | ||
| selectCommand(filteredCommands[selectedCommandIndex]); | ||
| } else if (filteredCommands.length > 0) { | ||
| selectCommand(filteredCommands[0]); | ||
| } | ||
| return; | ||
| } | ||
| if (e.key === 'Escape') { | ||
| e.preventDefault(); | ||
| setShowCommandMenu(false); |
There was a problem hiding this comment.
Keyboard slash-command selection never runs the command.
Enter/Tab in the menu call selectCommand, which only rewrites the textarea and closes the menu. executeCommand/history updates never fire, so keyboard users can’t trigger built‑ins/custom commands (they just send “/foo” to Claude). Please route keyboard selection through handleCommandSelect (or call executeCommand directly) so it matches the click path and records usage.
- if (e.key === 'Tab' || e.key === 'Enter') {
+ if (e.key === 'Tab' || e.key === 'Enter') {
e.preventDefault();
- if (selectedCommandIndex >= 0) {
- selectCommand(filteredCommands[selectedCommandIndex]);
- } else if (filteredCommands.length > 0) {
- selectCommand(filteredCommands[0]);
- }
+ const index = selectedCommandIndex >= 0 ? selectedCommandIndex : 0;
+ const command = filteredCommands[index];
+ if (command) {
+ handleCommandSelect(command, index, false);
+ }
return;
}(With this change selectCommand can also delegate to handleCommandSelect, keeping behavior consistent for both keyboard and mouse.)
Also applies to: 3421-3456
| <TaskIndicator | ||
| status={(() => { | ||
| const projectConfigured = project.taskmaster?.hasTaskmaster; | ||
| const mcpConfigured = mcpServerStatus?.hasMCPServer && mcpServerStatus?.isConfigured; | ||
| if (projectConfigured && mcpConfigured) return 'fully-configured'; | ||
| if (projectConfigured) return 'taskmaster-only'; | ||
| if (mcpConfigured) return 'mcp-only'; | ||
| return 'not-configured'; | ||
| })()} | ||
| })()} | ||
| size="xs" | ||
| className="flex-shrink-0 ml-2" | ||
| className="hidden md:inline-flex flex-shrink-0 ml-2" | ||
| /> |
There was a problem hiding this comment.
TaskIndicator never renders due to hidden class inside a md:hidden container
You're rendering TaskIndicator within a md:hidden block and also applying "hidden md:inline-flex" to the component. Result: it's hidden on mobile (hidden) and the entire block is hidden on md+ (md:hidden), so it never shows anywhere.
Fix by either:
- Move TaskIndicator into the Desktop Project Item block (lines 1141+) and keep "hidden md:inline-flex", or
- Keep it in the mobile block but drop the "hidden" so it shows on small screens.
Example (desktop placement suggestion):
- Insert TaskIndicator near the project name in the Desktop Item, with className="hidden md:inline-flex ml-2 flex-shrink-0".
This restores the intended “show on md+, hide on sm” behavior.
🤖 Prompt for AI Agents
In src/components/Sidebar.jsx around lines 1032 to 1043, TaskIndicator is placed
inside a container that has md:hidden while the component itself uses "hidden
md:inline-flex", which results in it never being visible; fix by moving the
TaskIndicator into the Desktop Project Item block (around line 1141) and keep
className="hidden md:inline-flex ml-2 flex-shrink-0" so it displays on md+ only,
or alternatively keep the current placement but remove the leading "hidden" from
its className to allow it to render on small screens (choose one approach and
update the JSX accordingly).
2 participants
Summary
Implements a slash command menu system with proper positioning and dark mode support.
Changes
CommandMenucomponent with grouped command display (Built-in, Project, User)/api/commands/*)position: fixedrelative to viewport.claude/commands/)~/.claude/commands/)Technical Details
position: absolutetoposition: fixedfor correct viewport positioningDependencies
Testing
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Bug Fixes