fix: crash prevention with bounded caches and session-scoped invalidation#170
fix: crash prevention with bounded caches and session-scoped invalidation#170psypeal wants to merge 9 commits intomatt1398:mainfrom
Conversation
# Conflicts: # src/renderer/components/chat/ContextBadge.tsx # src/renderer/utils/contextTracker.ts
# Conflicts: # src/renderer/components/chat/ContextBadge.tsx
sessionAnalyzer now reads pre-computed costUsd from calculateMetrics() instead of re-computing costs independently. Parent cost uses detail.metrics.costUsd, subagent cost uses proc.metrics.costUsd. This ensures the cost analysis panel and chat header always agree. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Electron's before-input-event preventDefault blocks both Chromium's built-in reload AND keydown propagation to the renderer. Route Ctrl+R through IPC instead: main process intercepts and sends session:refresh, preload bridges to renderer, store listener refreshes in place. Switch all refresh paths (IPC, refresh button, keyboard shortcut) to refreshSessionInPlace to avoid unmounting the scroll container, then dispatch a custom event to smoothly scroll to the bottom. Fixes matt1398#85
# Conflicts: # src/renderer/utils/sessionAnalyzer.ts # test/renderer/utils/sessionAnalyzer.test.ts
…hlighting - Add BashToolViewer with syntax-highlighted command input via CodeBlockViewer - Add CollapsibleOutputSection (collapsed by default) for tool output - Apply collapsible output to DefaultToolViewer and BashToolViewer - Add bash/zsh/fish keyword sets for syntax highlighting - Add keyword sets for c, cpp, java, kotlin, swift, lua, html, yaml - Fix markdown/html false type-coloring on capitalized words - Add markdown preview toggle to ReadToolViewer (defaults to preview) - Default WriteToolViewer to preview mode for markdown files - Extend comment detection for zsh, fish, yaml (#) and lua (--)
…tion - Add LRU eviction (500 entries) for ProjectScanner caches to prevent unbounded growth over long sessions - Add invalidateCachesForSession() for single-file cache invalidation instead of purging entire project on every file-change event - Add renderer memory monitoring (5-min interval, 2GB warning threshold) to detect leaks before they crash the renderer - Add crash recovery with retry caps, unresponsive watchdog, and crash logging to ~/.claude/claude-devtools-crash.log Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces robust crash handling and memory monitoring in the main process, including a new crash logging utility and automated renderer process recovery. The chat interface is enhanced with a specialized Bash tool viewer, collapsible output sections for tool results, and Markdown preview toggles for file-related tools. Additionally, syntax highlighting is expanded to support several new languages, and a session refresh mechanism is implemented via IPC. Feedback addresses a regression in Markdown syntax highlighting, improvements to error logging for crash reports, the use of constants for IPC channels, and fixes for cache eviction logic and UI state persistence.
|
|
||
| // If no highlighting support, return plain text as single-element array | ||
| if (keywords.size === 0 && !['json', 'css', 'html', 'bash', 'markdown'].includes(language)) { | ||
| if (keywords.size === 0 && !['json', 'css', 'bash'].includes(language)) { |
There was a problem hiding this comment.
Removing markdown from the exclusion list causes a regression in syntax highlighting for Markdown files. Since markdown is not present in the KEYWORDS map, keywords.size will be 0, and the function will now return the plain text line immediately, bypassing the basic tokenization (strings, numbers, comments) that was previously applied.
| if (keywords.size === 0 && !['json', 'css', 'bash'].includes(language)) { | |
| if (keywords.size === 0 && !['json', 'css', 'bash', 'markdown'].includes(language)) { |
| const entry = | ||
| `[${new Date().toISOString()}] ${label}\n` + | ||
| Object.entries(details) | ||
| .map(([k, v]) => ` ${k}: ${typeof v === 'string' ? v : JSON.stringify(v)}`) |
There was a problem hiding this comment.
Using JSON.stringify(v) for error details will result in an empty object {} if v is an Error instance, as error properties like message and stack are non-enumerable. It's better to explicitly handle Error objects to ensure useful information is logged.
| .map(([k, v]) => ` ${k}: ${typeof v === 'string' ? v : JSON.stringify(v)}`) | |
| .map(([k, v]) => ` ${k}: ${v instanceof Error ? v.stack ?? v.message : (typeof v === 'string' ? v : JSON.stringify(v))}`) |
| // hence the IPC bridge. | ||
| if ((input.control || input.meta) && !input.shift && input.key.toLowerCase() === 'r') { | ||
| event.preventDefault(); | ||
| mainWindow.webContents.send('session:refresh'); |
There was a problem hiding this comment.
The IPC channel name 'session:refresh' is hardcoded here as a string literal. It should use the SESSION_REFRESH constant defined in src/preload/constants/ipcChannels.ts to maintain consistency and avoid potential typos. You should add it to the duplicated constants list at the top of the file to follow the existing pattern.
| mainWindow.webContents.send('session:refresh'); | |
| mainWindow.webContents.send(SESSION_REFRESH); |
| private pruneCache<V>(cache: Map<string, V>): void { | ||
| if (cache.size <= ProjectScanner.MAX_CACHE_ENTRIES) return; | ||
| const excess = cache.size - ProjectScanner.MAX_CACHE_ENTRIES; | ||
| let removed = 0; | ||
| for (const key of cache.keys()) { | ||
| if (removed >= excess) break; | ||
| cache.delete(key); | ||
| removed++; | ||
| } | ||
| } |
There was a problem hiding this comment.
The pruneCache implementation follows a FIFO (First-In-First-Out) eviction strategy rather than LRU (Least Recently Used), as Map.keys() iterates in insertion order and the order is never updated when an entry is accessed via get(). If LRU behavior is intended, you should move the accessed key to the end of the map on every get operation. Otherwise, the documentation should be updated to reflect FIFO behavior.
| : undefined; | ||
|
|
||
| const isMarkdownFile = /\.mdx?$/i.test(filePath); | ||
| const [viewMode, setViewMode] = React.useState<'code' | 'preview'>(isMarkdownFile ? 'preview' : 'code'); |
There was a problem hiding this comment.
The viewMode state is initialized only when the component mounts. If the component is reused for a different linkedTool (e.g., when switching between files in the same session), the viewMode will persist from the previous file instead of resetting to the appropriate default for the new file type. This can lead to a confusing UX where a code file opens in 'preview' mode if the previous file was Markdown.
const isMarkdownFile = /\.mdx?$/i.test(filePath);
const [viewMode, setViewMode] = React.useState<'code' | 'preview'>(isMarkdownFile ? 'preview' : 'code');
// Reset view mode when file path changes
React.useEffect(() => {
setViewMode(isMarkdownFile ? 'preview' : 'code');
}, [filePath, isMarkdownFile]);
📝 WalkthroughWalkthroughAdded main process crash logging and bounded-recovery reload strategy with memory monitoring. Introduced session-scoped cache invalidation in ProjectScanner and FileWatcher. Added session refresh IPC channel for Ctrl+R/Cmd+R shortcuts. Implemented Bash tool viewer with collapsible output component. Expanded syntax highlighter language support and added Markdown preview toggles for file viewers. Changes
Possibly related PRs
Suggested labels
✨ Finishing Touches⚔️ Resolve merge conflicts
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: 6
🧹 Nitpick comments (6)
src/renderer/components/chat/items/linkedTool/ReadToolViewer.tsx (1)
61-100: Consider extracting the markdown mode toggle into a shared component.This block is now very similar to
WriteToolViewer, so a sharedMarkdownModeToggle(or hook + tiny component) would reduce duplication and prevent drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/chat/items/linkedTool/ReadToolViewer.tsx` around lines 61 - 100, Extract the repeated markdown view toggle UI from ReadToolViewer into a shared component (e.g., MarkdownModeToggle) and use it in both ReadToolViewer and WriteToolViewer: create a new component that accepts props {isMarkdownFile, viewMode, setViewMode} (or a hook that returns [viewMode, setViewMode]) and moves the two buttons and styling there, then replace the inline toggle block in ReadToolViewer (the fragment using isMarkdownFile, viewMode, setViewMode) with <MarkdownModeToggle .../> and do the same in WriteToolViewer so both reuse the same logic and visuals.src/main/services/discovery/ProjectScanner.ts (1)
109-122: FIFO eviction, not true LRU.The
pruneCache()method evicts entries based on Map insertion order, which is FIFO (first-in, first-out). True LRU eviction requires moving entries to the end of the map on cache hits (reads), so that frequently-accessed entries remain protected from eviction.For the use case here (session metadata caching), FIFO is likely acceptable since cache hits rarely repeat immediately—sessions are scanned, then users navigate away. However, if this becomes a concern, consider using a proper LRU cache like
lru-cacheor re-inserting on hits.Based on learnings: "Use LRU Cache to avoid re-parsing large JSONL files"—the current implementation is functionally a bounded FIFO cache rather than LRU.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/services/discovery/ProjectScanner.ts` around lines 109 - 122, pruneCache currently evicts by Map insertion order (FIFO) which is not true LRU; update the cache strategy used by ProjectScanner so reads refresh entry recency or replace the Map with an LRU implementation: either (1) on every read/update of the cache entry move it to the end of the Map (re-insert the key/value) so pruneCache continues to work as LRU, or (2) swap the Map for a standard LRU utility (e.g., the lru-cache package) and enforce MAX_CACHE_ENTRIES there; modify places that access the cache to use the chosen approach and keep pruneCache/ProjectScanner.MAX_CACHE_ENTRIES semantics consistent.src/main/index.ts (1)
715-745: Memory monitor interval is not cleared on window close.The
memoryMonitorIntervalis created but never cleared when the window is closed or destroyed. Whileunref()prevents it from blocking app exit, the interval will continue running and potentially throw errors when accessingmainWindow.webContentsafter the window is destroyed (though theif (!mainWindow || mainWindow.isDestroyed())guard catches this).For cleaner resource management, consider clearing the interval in the
mainWindow.on('closed')handler.🧹 Proposed fix to clear the interval
Add interval tracking and cleanup:
+ // Track interval for cleanup + let memoryMonitorInterval: ReturnType<typeof setInterval> | null = null; + // ... existing code ... - const memoryMonitorInterval = setInterval(() => { + memoryMonitorInterval = setInterval(() => { // ... existing code ... }, MEMORY_CHECK_INTERVAL_MS); memoryMonitorInterval.unref(); // In the 'closed' handler: mainWindow.on('closed', () => { mainWindow = null; + if (memoryMonitorInterval) { + clearInterval(memoryMonitorInterval); + memoryMonitorInterval = null; + } // ... existing cleanup ... });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/index.ts` around lines 715 - 745, The memoryMonitorInterval is never cleared when the window is closed; add cleanup to stop the interval to avoid background checks against a destroyed window. In the component that creates memoryMonitorInterval (the block using MEMORY_CHECK_INTERVAL_MS and calling mainWindow.webContents.getOSProcessId()), ensure you store the interval id in memoryMonitorInterval and call clearInterval(memoryMonitorInterval) from the mainWindow 'closed' (or 'close') event handler (e.g., inside mainWindow.on('closed', ...) or similar teardown logic) and remove/unregister any references so the interval is not left running after the window is destroyed.src/renderer/components/chat/items/LinkedToolItem.tsx (1)
44-45: Keep imports in guideline order.
Line 44adds a path-alias type import after relative imports. Group alias imports before relative imports to stay consistent.As per coding guidelines: organize imports as external packages, then path aliases, then relative imports.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/chat/items/LinkedToolItem.tsx` around lines 44 - 45, Reorder the imports in LinkedToolItem.tsx to follow the project guideline: place path-alias imports (e.g., "import type { StepVariant } from '@renderer/constants/stepVariants';" and "import type { LinkedToolItem as LinkedToolItemType } from '@renderer/types/groups';") before any relative imports; update the import block so external packages remain first, then the path-alias imports (StepVariant, LinkedToolItemType), and finally relative imports to restore the prescribed grouping order.src/renderer/components/chat/items/linkedTool/CollapsibleOutputSection.tsx (1)
30-40: Add explicit ARIA state to the toggle button.This is already keyboard-friendly; adding
aria-expandedandaria-controlswould improve screen-reader clarity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/chat/items/linkedTool/CollapsibleOutputSection.tsx` around lines 30 - 40, The toggle button in CollapsibleOutputSection lacks explicit ARIA state; update the button rendered in the CollapsibleOutputSection component to include aria-expanded tied to isExpanded and aria-controls pointing to the id of the collapsible panel (the element rendered when isExpanded is true). Ensure the panel element has a stable id (generate one using React's useId or a prop fallback) so the button's aria-controls references that id. Keep the existing onClick (setIsExpanded) behavior and only add these attributes to the <button> and the corresponding panel element to improve screen-reader clarity.src/renderer/utils/toolRendering/index.ts (1)
7-13: Avoid expanding renderer utility barrel exports.Adding
hasBashContenthere increases reliance on a renderer utils barrel; guideline direction is to import directly from specific files in renderer utilities.As per coding guidelines:
src/renderer/**/*.{ts,tsx}renderer utils/hooks/types should NOT use barrel exports.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/utils/toolRendering/index.ts` around lines 7 - 13, The barrel export index.ts is exposing hasBashContent which violates the renderer utils rule against using barrel exports; remove hasBashContent from the export list in src/renderer/utils/toolRendering/index.ts and update any callers to import hasBashContent directly from './toolContentChecks' (leave the other exports—hasEditContent, hasReadContent, hasSkillInstructions, hasWriteContent—if they remain allowed); ensure all references that previously imported hasBashContent from the barrel now import it from the specific module to comply with the renderer utils guideline.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/components/chat/ChatHistory.tsx`:
- Around line 380-387: The session-refresh-scroll-bottom listener is global and
causes every mounted ChatHistory to scroll; update the handler in the useEffect
so it only acts for the intended tab/pane by checking an identifier on the
incoming event (e.g., event.detail?.chatId or event.detail?.paneId) against this
component's id/active prop (e.g., props.chatId, props.sessionId or isActive
flag) before calling scrollToBottom; if the app dispatching the event doesn't
include an identifier, change the emitter to include one and then compare in the
handler (identify the handler, useEffect, scrollToBottom, and the
'session-refresh-scroll-bottom' event).
In `@src/renderer/components/chat/items/linkedTool/BashToolViewer.tsx`:
- Around line 25-27: The code in BashToolViewer currently unsafely casts
linkedTool.input with "as string"; replace this with a runtime type guard to
validate the shape before using fields. Add a reusable guard function (e.g.,
isBashToolInput) that checks input is a non-null object and that input.command
is a string (and optionally input.description is string), import it into
BashToolViewer, call it to narrow linkedTool.input, and only then read command
and description; update other viewers (EditToolViewer, WriteToolViewer) to use
corresponding guards for consistent validation.
In `@src/renderer/components/chat/items/LinkedToolItem.tsx`:
- Line 175: The LinkedToolItem component is passing an undeclared prop
variant={toolVariant} to BaseItem even though BaseItemProps does not define
variant; remove the prop from the JSX in LinkedToolItem (search for
LinkedToolItem and the BaseItem usage) so BaseItem is only given supported
props, or alternatively add variant to BaseItemProps and implement handling
inside BaseItem (update BaseItemProps and BaseItem component) if variant
behavior is required; prefer removing the prop here to resolve the TypeScript
error.
In `@src/renderer/hooks/useKeyboardShortcuts.ts`:
- Around line 263-268: The Promise.all call that runs
refreshSessionInPlace(selectedProjectId, selectedSessionId) and
fetchSessions(selectedProjectId) lacks error handling and can produce unhandled
promise rejections; wrap the Promise.all(...) chain with a .catch(...) to handle
errors (e.g., log with console.error or the app logger) and, if appropriate,
surface user feedback, while preserving the existing .then(...) that dispatches
the 'session-refresh-scroll-bottom' CustomEvent; reference the Promise.all call
and the functions refreshSessionInPlace, fetchSessions, and the dispatch of
'session-refresh-scroll-bottom' when making the change.
In `@src/renderer/store/index.ts`:
- Around line 278-283: The Promise.all call that invokes
state.refreshSessionInPlace and state.fetchSessions can reject and currently
leaves an unhandled rejection; update the code around the
Promise.all([...]).then(...) to append a .catch handler that explicitly handles
errors from refreshSessionInPlace and fetchSessions (e.g., log the error via
console.error or your app logger and/or surface a user-facing error) and ensure
post-refresh behavior is only triggered on success; reference the
refreshSessionInPlace and fetchSessions calls and the
'session-refresh-scroll-bottom' CustomEvent so you add the .catch after the
.then to avoid silent failures and unhandled rejections.
In `@src/renderer/utils/toolRendering/toolContentChecks.ts`:
- Around line 63-65: hasBashContent currently returns true for any truthy
linkedTool.input?.command (numbers, objects, booleans); change it into a proper
type guard by updating the function signature to a type predicate (e.g.,
hasBashContent(linkedTool: LinkedToolItem): linkedTool is LinkedToolItem & {
input: { command: string } }) and return only when typeof
linkedTool.input?.command === 'string' && linkedTool.input.command.trim().length
> 0 so callers can safely assume command is a non-empty string.
---
Nitpick comments:
In `@src/main/index.ts`:
- Around line 715-745: The memoryMonitorInterval is never cleared when the
window is closed; add cleanup to stop the interval to avoid background checks
against a destroyed window. In the component that creates memoryMonitorInterval
(the block using MEMORY_CHECK_INTERVAL_MS and calling
mainWindow.webContents.getOSProcessId()), ensure you store the interval id in
memoryMonitorInterval and call clearInterval(memoryMonitorInterval) from the
mainWindow 'closed' (or 'close') event handler (e.g., inside
mainWindow.on('closed', ...) or similar teardown logic) and remove/unregister
any references so the interval is not left running after the window is
destroyed.
In `@src/main/services/discovery/ProjectScanner.ts`:
- Around line 109-122: pruneCache currently evicts by Map insertion order (FIFO)
which is not true LRU; update the cache strategy used by ProjectScanner so reads
refresh entry recency or replace the Map with an LRU implementation: either (1)
on every read/update of the cache entry move it to the end of the Map (re-insert
the key/value) so pruneCache continues to work as LRU, or (2) swap the Map for a
standard LRU utility (e.g., the lru-cache package) and enforce MAX_CACHE_ENTRIES
there; modify places that access the cache to use the chosen approach and keep
pruneCache/ProjectScanner.MAX_CACHE_ENTRIES semantics consistent.
In `@src/renderer/components/chat/items/linkedTool/CollapsibleOutputSection.tsx`:
- Around line 30-40: The toggle button in CollapsibleOutputSection lacks
explicit ARIA state; update the button rendered in the CollapsibleOutputSection
component to include aria-expanded tied to isExpanded and aria-controls pointing
to the id of the collapsible panel (the element rendered when isExpanded is
true). Ensure the panel element has a stable id (generate one using React's
useId or a prop fallback) so the button's aria-controls references that id. Keep
the existing onClick (setIsExpanded) behavior and only add these attributes to
the <button> and the corresponding panel element to improve screen-reader
clarity.
In `@src/renderer/components/chat/items/linkedTool/ReadToolViewer.tsx`:
- Around line 61-100: Extract the repeated markdown view toggle UI from
ReadToolViewer into a shared component (e.g., MarkdownModeToggle) and use it in
both ReadToolViewer and WriteToolViewer: create a new component that accepts
props {isMarkdownFile, viewMode, setViewMode} (or a hook that returns [viewMode,
setViewMode]) and moves the two buttons and styling there, then replace the
inline toggle block in ReadToolViewer (the fragment using isMarkdownFile,
viewMode, setViewMode) with <MarkdownModeToggle .../> and do the same in
WriteToolViewer so both reuse the same logic and visuals.
In `@src/renderer/components/chat/items/LinkedToolItem.tsx`:
- Around line 44-45: Reorder the imports in LinkedToolItem.tsx to follow the
project guideline: place path-alias imports (e.g., "import type { StepVariant }
from '@renderer/constants/stepVariants';" and "import type { LinkedToolItem as
LinkedToolItemType } from '@renderer/types/groups';") before any relative
imports; update the import block so external packages remain first, then the
path-alias imports (StepVariant, LinkedToolItemType), and finally relative
imports to restore the prescribed grouping order.
In `@src/renderer/utils/toolRendering/index.ts`:
- Around line 7-13: The barrel export index.ts is exposing hasBashContent which
violates the renderer utils rule against using barrel exports; remove
hasBashContent from the export list in src/renderer/utils/toolRendering/index.ts
and update any callers to import hasBashContent directly from
'./toolContentChecks' (leave the other exports—hasEditContent, hasReadContent,
hasSkillInstructions, hasWriteContent—if they remain allowed); ensure all
references that previously imported hasBashContent from the barrel now import it
from the specific module to comply with the renderer utils guideline.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0be83daf-cb95-431a-858e-1a8b9929ab66
📒 Files selected for processing (22)
src/main/index.tssrc/main/services/discovery/ProjectScanner.tssrc/main/services/infrastructure/FileWatcher.tssrc/main/services/infrastructure/ServiceContext.tssrc/preload/constants/ipcChannels.tssrc/preload/index.tssrc/renderer/api/httpClient.tssrc/renderer/components/chat/ChatHistory.tsxsrc/renderer/components/chat/items/LinkedToolItem.tsxsrc/renderer/components/chat/items/linkedTool/BashToolViewer.tsxsrc/renderer/components/chat/items/linkedTool/CollapsibleOutputSection.tsxsrc/renderer/components/chat/items/linkedTool/DefaultToolViewer.tsxsrc/renderer/components/chat/items/linkedTool/ReadToolViewer.tsxsrc/renderer/components/chat/items/linkedTool/WriteToolViewer.tsxsrc/renderer/components/chat/items/linkedTool/index.tssrc/renderer/components/chat/viewers/syntaxHighlighter.tssrc/renderer/components/layout/TabBar.tsxsrc/renderer/hooks/useKeyboardShortcuts.tssrc/renderer/store/index.tssrc/renderer/utils/toolRendering/index.tssrc/renderer/utils/toolRendering/toolContentChecks.tssrc/shared/types/api.ts
| // Listen for session-refresh-scroll-bottom events (from Ctrl+R / refresh button) | ||
| useEffect(() => { | ||
| const handler = (): void => { | ||
| scrollToBottom('smooth'); | ||
| }; | ||
| window.addEventListener('session-refresh-scroll-bottom', handler); | ||
| return () => window.removeEventListener('session-refresh-scroll-bottom', handler); | ||
| }, [scrollToBottom]); |
There was a problem hiding this comment.
Scope refresh-scroll handling to the intended tab/pane.
This handler runs for every mounted ChatHistory, so one refresh event can scroll unrelated views to bottom and disrupt reading position.
💡 Proposed fix
useEffect(() => {
const handler = (): void => {
- scrollToBottom('smooth');
+ if (!isThisTabActive) return;
+ scrollToBottom('smooth');
};
window.addEventListener('session-refresh-scroll-bottom', handler);
return () => window.removeEventListener('session-refresh-scroll-bottom', handler);
- }, [scrollToBottom]);
+ }, [isThisTabActive, scrollToBottom]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/chat/ChatHistory.tsx` around lines 380 - 387, The
session-refresh-scroll-bottom listener is global and causes every mounted
ChatHistory to scroll; update the handler in the useEffect so it only acts for
the intended tab/pane by checking an identifier on the incoming event (e.g.,
event.detail?.chatId or event.detail?.paneId) against this component's id/active
prop (e.g., props.chatId, props.sessionId or isActive flag) before calling
scrollToBottom; if the app dispatching the event doesn't include an identifier,
change the emitter to include one and then compare in the handler (identify the
handler, useEffect, scrollToBottom, and the 'session-refresh-scroll-bottom'
event).
| const command = linkedTool.input.command as string; | ||
| const description = linkedTool.input.description as string | undefined; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/renderer/components/chat/items/linkedTool/BashToolViewer.tsx | head -35Repository: matt1398/claude-devtools
Length of output: 1329
🏁 Script executed:
# Search for type guard utilities in the codebase
fd -t f "isXxx" --iglob "*.ts" --iglob "*.tsx" src/Repository: matt1398/claude-devtools
Length of output: 270
🏁 Script executed:
# Look for existing type guard patterns in renderer utils
rg "^export const is[A-Z]" src/renderer/ -A 2 | head -30Repository: matt1398/claude-devtools
Length of output: 276
🏁 Script executed:
# Check if there are validation utilities for linked tools
rg "linkedTool" src/renderer/ -t ts -t tsx -l | head -10Repository: matt1398/claude-devtools
Length of output: 95
🏁 Script executed:
# Find LinkedToolItem type definition
rg "type LinkedToolItem" src/renderer/ -A 10Repository: matt1398/claude-devtools
Length of output: 50
🏁 Script executed:
# Search for type guards with isXxx pattern
rg "export const is[A-Z]" src/ -t ts | head -20Repository: matt1398/claude-devtools
Length of output: 160
🏁 Script executed:
# Check for tool input validation patterns
rg "linkedTool\.input" src/renderer/ -B 2 -A 2 | head -40Repository: matt1398/claude-devtools
Length of output: 3361
🏁 Script executed:
# Check the types/groups file for LinkedToolItem
cat -n src/renderer/types/groups.ts | head -100Repository: matt1398/claude-devtools
Length of output: 3494
🏁 Script executed:
# Search for LinkedToolItem interface
rg "interface LinkedToolItem" src/renderer/types/Repository: matt1398/claude-devtools
Length of output: 130
🏁 Script executed:
# Get the full types/groups.ts file
wc -l src/renderer/types/groups.tsRepository: matt1398/claude-devtools
Length of output: 100
🏁 Script executed:
# Search for LinkedToolItem definition in all types files
rg "LinkedToolItem" src/renderer/types/ -A 15 | head -60Repository: matt1398/claude-devtools
Length of output: 3120
🏁 Script executed:
# Check other tool viewers for validation patterns
cat -n src/renderer/components/chat/items/linkedTool/WriteToolViewer.tsx | head -40Repository: matt1398/claude-devtools
Length of output: 1909
🏁 Script executed:
# Check EditToolViewer
cat -n src/renderer/components/chat/items/linkedTool/EditToolViewer.tsx | head -40Repository: matt1398/claude-devtools
Length of output: 1533
🏁 Script executed:
# Search for validation utilities or type guards related to tools
rg "function is" src/renderer/utils/ -A 3Repository: matt1398/claude-devtools
Length of output: 1736
Create type guards for tool input validation instead of unsafe casts.
Lines 25-26 use unsafe as string casts on untyped input fields. Per coding guidelines, runtime validation should use isXxx type guards. This pattern appears across multiple tool viewers (BashToolViewer, EditToolViewer, WriteToolViewer).
Create reusable type guards like isBashToolInput(input): input is { command: string; description?: string } and use them consistently:
Suggested approach
- const command = linkedTool.input.command as string;
- const description = linkedTool.input.description as string | undefined;
+ if (!isBashToolInput(linkedTool.input)) {
+ return null;
+ }
+ const { command, description } = linkedTool.input;
+ if (command.trim().length === 0) return null;Define in a shared validation module (e.g., utils/toolRendering/toolInputGuards.ts):
export function isBashToolInput(input: unknown): input is { command: string; description?: string } {
return typeof input === 'object' && input !== null &&
typeof (input as Record<string, unknown>).command === 'string';
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/chat/items/linkedTool/BashToolViewer.tsx` around
lines 25 - 27, The code in BashToolViewer currently unsafely casts
linkedTool.input with "as string"; replace this with a runtime type guard to
validate the shape before using fields. Add a reusable guard function (e.g.,
isBashToolInput) that checks input is a non-null object and that input.command
is a string (and optionally input.description is string), import it into
BashToolViewer, call it to narrow linkedTool.input, and only then read command
and description; update other viewers (EditToolViewer, WriteToolViewer) to use
corresponding guards for consistent validation.
| highlightClasses={highlightClasses} | ||
| highlightStyle={highlightStyle} | ||
| notificationDotColor={notificationDotColor} | ||
| variant={toolVariant} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether BaseItemProps declares `variant` and where BaseItem is called with it.
rg -nP --type=tsx -C3 'interface\s+BaseItemProps|variant\??\s*:' src/renderer/components/chat/items/BaseItem.tsx
rg -nP --type=tsx -C2 '<BaseItem[^>]*\bvariant=' src/renderer/components/chat/items/LinkedToolItem.tsxRepository: matt1398/claude-devtools
Length of output: 123
🏁 Script executed:
#!/bin/bash
# Search for BaseItemProps interface and variant usage without type restrictions
rg -nA5 'interface\s+BaseItemProps' src/renderer/components/chat/items/BaseItem.tsxRepository: matt1398/claude-devtools
Length of output: 295
🏁 Script executed:
#!/bin/bash
# Read the LinkedToolItem.tsx file to see line 175 and surrounding context
wc -l src/renderer/components/chat/items/LinkedToolItem.tsxRepository: matt1398/claude-devtools
Length of output: 125
🏁 Script executed:
#!/bin/bash
# List files in the items directory to confirm they exist
ls -la src/renderer/components/chat/items/Repository: matt1398/claude-devtools
Length of output: 913
🏁 Script executed:
#!/bin/bash
# Read the full BaseItemProps interface definition
sed -n '15,40p' src/renderer/components/chat/items/BaseItem.tsxRepository: matt1398/claude-devtools
Length of output: 1012
🏁 Script executed:
#!/bin/bash
# Read line 175 and surrounding context from LinkedToolItem.tsx
sed -n '170,180p' src/renderer/components/chat/items/LinkedToolItem.tsxRepository: matt1398/claude-devtools
Length of output: 459
🏁 Script executed:
#!/bin/bash
# Continue reading BaseItemProps to see if variant is declared later
sed -n '40,60p' src/renderer/components/chat/items/BaseItem.tsxRepository: matt1398/claude-devtools
Length of output: 691
🏁 Script executed:
#!/bin/bash
# Read more context around line 175 to see which component the variant prop is on
sed -n '160,180p' src/renderer/components/chat/items/LinkedToolItem.tsxRepository: matt1398/claude-devtools
Length of output: 808
🏁 Script executed:
#!/bin/bash
# Check BaseItem component render/export signature
sed -n '1,100p' src/renderer/components/chat/items/BaseItem.tsx | grep -A 30 'export\|function\|const.*BaseItem'Repository: matt1398/claude-devtools
Length of output: 2455
🏁 Script executed:
#!/bin/bash
# Read exact lines around 175 to identify the opening tag
sed -n '165,178p' src/renderer/components/chat/items/LinkedToolItem.tsxRepository: matt1398/claude-devtools
Length of output: 595
🏁 Script executed:
#!/bin/bash
# Read earlier lines to see the opening tag of the component
sed -n '155,175p' src/renderer/components/chat/items/LinkedToolItem.tsxRepository: matt1398/claude-devtools
Length of output: 709
Remove undeclared variant prop from BaseItem component.
Line 175 passes variant={toolVariant} to <BaseItem>, but BaseItemProps does not include this prop. The BaseItem component will not accept or use this prop, causing a TypeScript error.
Fix
- variant={toolVariant}📝 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.
| variant={toolVariant} | |
| {/* variant prop removed - not supported by BaseItemProps */} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/chat/items/LinkedToolItem.tsx` at line 175, The
LinkedToolItem component is passing an undeclared prop variant={toolVariant} to
BaseItem even though BaseItemProps does not define variant; remove the prop from
the JSX in LinkedToolItem (search for LinkedToolItem and the BaseItem usage) so
BaseItem is only given supported props, or alternatively add variant to
BaseItemProps and implement handling inside BaseItem (update BaseItemProps and
BaseItem component) if variant behavior is required; prefer removing the prop
here to resolve the TypeScript error.
| void Promise.all([ | ||
| fetchSessionDetail(selectedProjectId, selectedSessionId), | ||
| refreshSessionInPlace(selectedProjectId, selectedSessionId), | ||
| fetchSessions(selectedProjectId), | ||
| ]); | ||
| ]).then(() => { | ||
| window.dispatchEvent(new CustomEvent('session-refresh-scroll-bottom')); | ||
| }); |
There was a problem hiding this comment.
Add error handling for the refresh shortcut async flow.
This Promise chain can reject without a handler, causing unhandled rejection noise and harder failure diagnosis.
💡 Proposed fix
void Promise.all([
refreshSessionInPlace(selectedProjectId, selectedSessionId),
fetchSessions(selectedProjectId),
- ]).then(() => {
- window.dispatchEvent(new CustomEvent('session-refresh-scroll-bottom'));
- });
+ ])
+ .then(() => {
+ window.dispatchEvent(new CustomEvent('session-refresh-scroll-bottom'));
+ })
+ .catch((error: unknown) => {
+ console.error('[KeyboardShortcuts] Session refresh failed:', error);
+ });📝 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.
| void Promise.all([ | |
| fetchSessionDetail(selectedProjectId, selectedSessionId), | |
| refreshSessionInPlace(selectedProjectId, selectedSessionId), | |
| fetchSessions(selectedProjectId), | |
| ]); | |
| ]).then(() => { | |
| window.dispatchEvent(new CustomEvent('session-refresh-scroll-bottom')); | |
| }); | |
| void Promise.all([ | |
| refreshSessionInPlace(selectedProjectId, selectedSessionId), | |
| fetchSessions(selectedProjectId), | |
| ]) | |
| .then(() => { | |
| window.dispatchEvent(new CustomEvent('session-refresh-scroll-bottom')); | |
| }) | |
| .catch((error: unknown) => { | |
| console.error('[KeyboardShortcuts] Session refresh failed:', error); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/hooks/useKeyboardShortcuts.ts` around lines 263 - 268, The
Promise.all call that runs refreshSessionInPlace(selectedProjectId,
selectedSessionId) and fetchSessions(selectedProjectId) lacks error handling and
can produce unhandled promise rejections; wrap the Promise.all(...) chain with a
.catch(...) to handle errors (e.g., log with console.error or the app logger)
and, if appropriate, surface user feedback, while preserving the existing
.then(...) that dispatches the 'session-refresh-scroll-bottom' CustomEvent;
reference the Promise.all call and the functions refreshSessionInPlace,
fetchSessions, and the dispatch of 'session-refresh-scroll-bottom' when making
the change.
| void Promise.all([ | ||
| state.refreshSessionInPlace(activeTab.projectId, activeTab.sessionId), | ||
| state.fetchSessions(activeTab.projectId), | ||
| ]).then(() => { | ||
| window.dispatchEvent(new CustomEvent('session-refresh-scroll-bottom')); | ||
| }); |
There was a problem hiding this comment.
Handle refresh Promise rejections explicitly.
If either async call fails, this currently produces an unhandled rejection and silently skips post-refresh behavior.
💡 Proposed fix
- void Promise.all([
- state.refreshSessionInPlace(activeTab.projectId, activeTab.sessionId),
- state.fetchSessions(activeTab.projectId),
- ]).then(() => {
- window.dispatchEvent(new CustomEvent('session-refresh-scroll-bottom'));
- });
+ void Promise.all([
+ state.refreshSessionInPlace(activeTab.projectId, activeTab.sessionId),
+ state.fetchSessions(activeTab.projectId),
+ ])
+ .then(() => {
+ window.dispatchEvent(new CustomEvent('session-refresh-scroll-bottom'));
+ })
+ .catch((error: unknown) => {
+ console.error('[Store] Session refresh failed:', error);
+ });📝 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.
| void Promise.all([ | |
| state.refreshSessionInPlace(activeTab.projectId, activeTab.sessionId), | |
| state.fetchSessions(activeTab.projectId), | |
| ]).then(() => { | |
| window.dispatchEvent(new CustomEvent('session-refresh-scroll-bottom')); | |
| }); | |
| void Promise.all([ | |
| state.refreshSessionInPlace(activeTab.projectId, activeTab.sessionId), | |
| state.fetchSessions(activeTab.projectId), | |
| ]) | |
| .then(() => { | |
| window.dispatchEvent(new CustomEvent('session-refresh-scroll-bottom')); | |
| }) | |
| .catch((error: unknown) => { | |
| console.error('[Store] Session refresh failed:', error); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/store/index.ts` around lines 278 - 283, The Promise.all call
that invokes state.refreshSessionInPlace and state.fetchSessions can reject and
currently leaves an unhandled rejection; update the code around the
Promise.all([...]).then(...) to append a .catch handler that explicitly handles
errors from refreshSessionInPlace and fetchSessions (e.g., log the error via
console.error or your app logger and/or surface a user-facing error) and ensure
post-refresh behavior is only triggered on success; reference the
refreshSessionInPlace and fetchSessions calls and the
'session-refresh-scroll-bottom' CustomEvent so you add the .catch after the
.then to avoid silent failures and unhandled rejections.
| export function hasBashContent(linkedTool: LinkedToolItem): boolean { | ||
| return !!linkedTool.input?.command; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat src/renderer/utils/toolRendering/toolContentChecks.tsRepository: matt1398/claude-devtools
Length of output: 2075
🏁 Script executed:
cd src/renderer && find . -name 'groups.ts' -o -name 'groups.tsx' | head -5Repository: matt1398/claude-devtools
Length of output: 85
🏁 Script executed:
cat src/renderer/types/groups.ts | head -100Repository: matt1398/claude-devtools
Length of output: 2794
🏁 Script executed:
rg "export (interface|type) LinkedToolItem" src/renderer/types/groups.ts -A 20Repository: matt1398/claude-devtools
Length of output: 726
🏁 Script executed:
rg "!!.*\?" src/renderer/utils/toolRendering/toolContentChecks.tsRepository: matt1398/claude-devtools
Length of output: 105
🏁 Script executed:
rg "!!.*\?" src/renderer/utils/ -t ts -t tsx --max-count 10Repository: matt1398/claude-devtools
Length of output: 95
🏁 Script executed:
rg "!!.*\?" src/renderer/utils/ -t ts --max-count 10Repository: matt1398/claude-devtools
Length of output: 159
Use a proper type guard for bash command validation.
The current implementation accepts any truthy command value (objects, numbers, booleans), which could incorrectly route to the Bash viewer and cause unsafe type casts downstream. Implement a type guard with proper string validation.
Suggested fix
+function isBashInput(input: Record<string, unknown>): input is { command: string } {
+ return typeof input.command === 'string' && input.command.trim().length > 0;
+}
+
export function hasBashContent(linkedTool: LinkedToolItem): boolean {
- return !!linkedTool.input?.command;
+ return isBashInput(linkedTool.input);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/utils/toolRendering/toolContentChecks.ts` around lines 63 - 65,
hasBashContent currently returns true for any truthy linkedTool.input?.command
(numbers, objects, booleans); change it into a proper type guard by updating the
function signature to a type predicate (e.g., hasBashContent(linkedTool:
LinkedToolItem): linkedTool is LinkedToolItem & { input: { command: string } })
and return only when typeof linkedTool.input?.command === 'string' &&
linkedTool.input.command.trim().length > 0 so callers can safely assume command
is a non-empty string.
1 participant
Summary
sessionMetadataCache,sessionPreviewCache,contentPresenceCache) to prevent unbounded main process heap growthinvalidateCachesForSession()— a single busy agent session no longer evicts all sibling session cachesapp.getAppMetrics()— logsRENDERER_MEMORY_WARNINGat 2GB threshold to crash log~/.claude/claude-devtools-crash.logTest plan
pnpm typecheck— no new errors (pre-existing errors inLinkedToolItem.tsxunrelated to this PR)pnpm test— all 652 tests passpnpm lint— all changed files pass lint~/.claude/claude-devtools-crash.logfor memory monitoring entries🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Improvements