Refactor: Frontend architecture to FSD-Lite structure#13
Conversation
Migration details: - Moved 5 UI components (WelcomeView, NewWorkspaceModal, CloneRepositoryModal, RepoGroup, WorkspaceItem) - Moved API service (repo.service.ts → repository.service.ts) - Moved query hooks (useRepoQueries.ts → repository.queries.ts) - Moved types (repo.types.ts → types.ts with Repo, Stats) Key fixes: - Fixed repository.queries.ts to import from local './repository.service' - Fixed repository.service.ts to import types from '../types' - Fixed RepoGroup.tsx to import workspace types from '@/shared/types' - Fixed useWorkspaceQueries.ts to import RepoService from feature - Removed RepoService from services/index.ts - Removed duplicate useRepoQueries from hooks/queries/index.ts - Added backward compatibility re-exports in shared/types and hooks/queries TypeScript: ✅ 0 errors Status: Phase 7 complete, ready for Phase 8 (workspace feature) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Migration details: - Extracted FileChangesPanel component from Dashboard (lines 595-679) - Moved 2 UI components (DiffModal, FileChangesPanel - extracted) - Moved API layer (workspace.queries.ts, workspace.service.ts) - Moved store (workspaceStore.ts) - Moved types (workspace.types.ts with 8 types) Key achievements: - Created FileChangesPanel with internal data fetching and file click handling - Split type imports (local vs shared) in queries and service - Added backward compatibility re-exports in shared/types, hooks/queries, stores - Updated Dashboard to use new workspace feature components - Removed handleFileClick, fileChanges, devServers from Dashboard Files moved/created: - UI: DiffModal.tsx (moved), FileChangesPanel.tsx (created) - API: workspace.queries.ts, workspace.service.ts - Store: workspaceStore.ts - Types: Workspace, WorkspaceState, SessionStatus, RepoGroup, DiffStats, FileChange, FileEdit, FileChangeGroup TypeScript: ✅ 0 errors Status: Phase 8 complete, ready for Phase 9 (session feature - 40+ files) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
zvadaadam
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/components/ui/EmptyState.tsx (1)
40-40: Constrain transition to transform, add ease and reduced‑motion override.Keeps animation performant and aligns with timing/easing and reduced‑motion rules. Two nits applied below.
As per coding guidelines.
- <div className="w-16 h-16 mb-5 text-muted-foreground/50 [@media(hover:hover)and(pointer:fine)]:transition-all [@media(hover:hover)and(pointer:fine)]:duration-200 [@media(hover:hover)and(pointer:fine)]:hover:scale-105"> + <div className="w-16 h-16 mb-5 text-muted-foreground/50 [@media(hover:hover)and(pointer:fine)]:transition-transform [@media(hover:hover)and(pointer:fine)]:duration-200 [@media(hover:hover)and(pointer:fine)]:ease [@media(hover:hover)and(pointer:fine)]:hover:scale-105 motion-reduce:transition-none motion-reduce:hover:scale-100">
🧹 Nitpick comments (3)
src/platform/tauri/socket/SocketClient.ts (1)
35-39: Add explicit return type annotation.The
onMessagemethod should declare its return type asPromise<UnlistenFn>for better type safety and developer experience.Apply this diff to add the return type:
- onMessage(callback: (data: string) => void) { + onMessage(callback: (data: string) => void): Promise<UnlistenFn> { return listen('socket_message', (event) => { callback(event.payload as string); }); }You'll also need to import the
UnlistenFntype:-import { invoke, listen } from '../invoke'; +import { invoke, listen, UnlistenFn } from '../invoke';src/features/session/ui/tools/renderers/KillShellToolRenderer.tsx (2)
44-44: Replace hardcoded colors with semantic tokens for consistency.While line 27 was updated to use semantic color tokens, these icon colors still use hardcoded values. For consistency with the refactor and to align with coding guidelines, consider using semantic tokens.
Apply this diff:
- <XCircle className="w-4 h-4 text-red-600 dark:text-red-400" aria-hidden="true" /> + <XCircle className="w-4 h-4 text-destructive" aria-hidden="true" />And similarly for line 59:
- <Terminal className="w-3 h-3 text-red-600 dark:text-red-400" aria-hidden="true" /> + <Terminal className="w-3 h-3 text-destructive" aria-hidden="true" />As per coding guidelines
Also applies to: 59-59
68-72: Consider using transform-based animation instead of height.The current animation uses height, which can cause performance issues. Per coding guidelines, prefer animating
transformandopacity. Also, Framer Motion guidelines recommend defaulting to spring animations.Consider this approach:
<motion.div - initial={{ height: 0, opacity: 0 }} - animate={{ height: 'auto', opacity: 1 }} - exit={{ height: 0, opacity: 0 }} - transition={{ duration: 0.2, ease: [0.23, 1, 0.32, 1] }} + initial={{ scaleY: 0, opacity: 0 }} + animate={{ scaleY: 1, opacity: 1 }} + exit={{ scaleY: 0, opacity: 0 }} + transition={{ type: 'spring', stiffness: 300, damping: 25 }} + style={{ transformOrigin: 'top' }} className="overflow-hidden" >As per coding guidelines
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (10)
src/components/ui/EmptyState.tsx(2 hunks)src/features/session/ui/tools/renderers/BashToolRenderer.tsx(3 hunks)src/features/session/ui/tools/renderers/DefaultToolRenderer.tsx(3 hunks)src/features/session/ui/tools/renderers/KillShellToolRenderer.tsx(2 hunks)src/features/session/ui/tools/renderers/TodoWriteToolRenderer.tsx(3 hunks)src/features/session/ui/tools/renderers/WebSearchToolRenderer.tsx(4 hunks)src/features/workspace/types.ts(1 hunks)src/features/workspace/ui/FileChangesPanel.tsx(1 hunks)src/platform/tauri/invoke.ts(1 hunks)src/platform/tauri/socket/SocketClient.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/features/workspace/types.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/features/session/ui/tools/renderers/BashToolRenderer.tsx
- src/platform/tauri/invoke.ts
- src/features/session/ui/tools/renderers/WebSearchToolRenderer.tsx
- src/features/workspace/ui/FileChangesPanel.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{tsx,jsx,css}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{tsx,jsx,css}: Avoid hardcoding colors; always use color tokens from Tailwind config
Always use font tokens from the Tailwind config (avoid hardcoded font sizes/families)
Use consistent paddings with a default of 16px unless there’s a strong reason otherwise
Files:
src/features/session/ui/tools/renderers/KillShellToolRenderer.tsxsrc/features/session/ui/tools/renderers/TodoWriteToolRenderer.tsxsrc/features/session/ui/tools/renderers/DefaultToolRenderer.tsxsrc/components/ui/EmptyState.tsx
**/*.{css,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{css,tsx,jsx}: Default to ease-out for most animations
Animations should be shorter than 1s; most between 0.2s and 0.3s
Do not use built-in CSS easings excepteaseorlinear
Prefer ease-out for entering/user-initiated interactions; use ease-in-out for in-place movements; generally avoid ease-in
Useeasewith 200ms duration for simple hover transitions (color, background-color, opacity)
Disable hover transitions on touch devices using@media (hover: hover) and (pointer: fine)
Respect prefers-reduced-motion: disable transform-based animations when reduced motion is requested
Animate elements from their trigger by setting an appropriate transform-origin
Prefer animating opacity and transform; avoid animating positional properties like top/left
Do not animate drag gestures using CSS variables
Do not animate blur values higher than 20px
Usewill-changesparingly and only for transform, opacity, clipPath, or filter
Files:
src/features/session/ui/tools/renderers/KillShellToolRenderer.tsxsrc/features/session/ui/tools/renderers/TodoWriteToolRenderer.tsxsrc/features/session/ui/tools/renderers/DefaultToolRenderer.tsxsrc/components/ui/EmptyState.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{tsx,jsx}: When using Framer Motion, use transform rather than x/y for hardware-accelerated animations
Default to spring animations with Framer Motion
Avoid bouncy spring animations unless working with drag gestures
Files:
src/features/session/ui/tools/renderers/KillShellToolRenderer.tsxsrc/features/session/ui/tools/renderers/TodoWriteToolRenderer.tsxsrc/features/session/ui/tools/renderers/DefaultToolRenderer.tsxsrc/components/ui/EmptyState.tsx
🧬 Code graph analysis (1)
src/platform/tauri/socket/SocketClient.ts (2)
src/platform/tauri/invoke.ts (2)
invoke(18-25)listen(31-39)src/platform/tauri/index.ts (1)
invoke(6-6)
🔇 Additional comments (7)
src/features/session/ui/tools/renderers/DefaultToolRenderer.tsx (1)
12-13: LGTM! Clean refactoring to FSD-Lite structure with improved color semantics.The changes correctly update import paths to the new centralized locations and migrate MCP tool styling from purple-based colors to semantic color tokens (
secondary/50,secondary/5,secondary-foreground). This improves theme consistency and maintainability while adhering to the coding guidelines for using Tailwind color tokens.Also applies to: 54-54, 72-72
src/platform/tauri/socket/SocketClient.ts (1)
8-8: Platform abstraction properly implemented.The previous review concern about direct Tauri imports has been addressed. The code now correctly uses the platform wrapper from
../invoke, consistent with the documented FSD-Lite architecture.src/features/session/ui/tools/renderers/KillShellToolRenderer.tsx (2)
12-13: LGTM! Import paths updated correctly.The import path changes align with the FSD-Lite refactor, moving shared utilities to
@/shared/lib/utilsand consolidating types intochat-types.
27-27: Excellent use of semantic color tokens!The change from hardcoded colors to semantic tokens (
border-l-destructive/50 bg-destructive/5) improves maintainability and aligns with the coding guidelines.src/features/session/ui/tools/renderers/TodoWriteToolRenderer.tsx (2)
12-13: LGTM! Import paths correctly updated for FSD-Lite architecture.The import path changes align with the feature-sliced design refactor, moving utilities to the shared layer and consolidating types into chat-types.
62-62: LGTM! Styling updates follow color token guidelines.The transition from hardcoded purple colors to semantic Tailwind tokens (secondary/50, secondary/5, secondary-foreground) improves maintainability and aligns with the coding guideline to avoid hardcoded colors.
Also applies to: 79-79
src/components/ui/EmptyState.tsx (1)
2-7: Import paths verified and correctly configured.✓ Path aliases exist in both tsconfig.json and vite.config.ts
✓cnexported from src/shared/lib/utils.ts
✓ EmptyStateContainer, EmptyStateTitle, EmptyStateDescription exported from src/shared/components barrelThe import refactor is complete and follows the FSD-Lite architecture as intended.
|
@CodeRabbit full review |
|
@codex review |
✅ Actions performedFull review triggered. |
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 (3)
src/features/terminal/ui/Terminal.tsx (1)
100-107: Potential duplicate event handler registration.The
onDatahandler is registered twice: once in the firstuseEffect(lines 100-107) and again in the seconduseEffect(lines 147-158). This can lead to duplicate writes being sent to the PTY when the user types, as xterm will call both handlers for each keystroke.Consider removing the first
onDataregistration and keeping only the second one that properly tracksisReadychanges:- // Handle terminal input - xterm.onData((data) => { - if (isReady) { - const bytes = new TextEncoder().encode(data); - ptyCommands.write(id, Array.from(bytes)).catch((err) => { - console.error('Failed to write to PTY:', err); - }); - } - }); - // Listen for PTY dataAlso applies to: 147-158
src/features/session/ui/MessageItem.tsx (1)
52-62: Bug: Non‑array content is incorrectly filtered out.Early return triggers for any non‑array content, making the JSON fallback unreachable. Restrict skipping to only-empty or only-tool_result arrays.
- const hasRenderableContent = Array.isArray(contentBlocks) && - contentBlocks.length > 0 && - contentBlocks.some((block: any) => block.type !== 'tool_result'); + const isArray = Array.isArray(contentBlocks); + const onlyToolResults = + isArray && + contentBlocks.length > 0 && + contentBlocks.every((block: any) => block?.type === 'tool_result'); + const isEmpty = + (isArray && contentBlocks.length === 0) || + (!isArray && (contentBlocks == null || String(contentBlocks).trim() === '')); + const hasRenderableContent = !(onlyToolResults || isEmpty); - // Skip messages without renderable content (see comment above for details) + // Skip messages that are empty or contain only tool_result blocks (see comment above) if (!hasRenderableContent) { if (import.meta.env.DEV) { console.log(`[MessageItem] Skipping empty message ${message.id} (${message.role})`); } return null; }src/features/browser/hooks/useBrowser.ts (1)
74-86: Add timeout to Web-mode health check to avoid hangsfetch('http://localhost:3000/health') in startServer has no timeout, unlike checkStatus(). A stalled socket can hang the UI promise.
Apply:
- const response = await fetch('http://localhost:3000/health'); + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 3000); + let response: Response; + try { + response = await fetch('http://localhost:3000/health', { signal: controller.signal }); + } finally { + clearTimeout(timeoutId); + }
♻️ Duplicate comments (2)
REFACTORING_PLAN_v2.md (1)
1-10: Past review concern has been resolved.The previous review flagged a critical mismatch between the PR title/description and the actual changes. This has been addressed—the PR title now correctly reads "Refactor: Frontend architecture to FSD-Lite structure (PR #13)" and the description accurately reflects the 141-file architectural refactoring scope.
src/features/session/ui/blocks/BlockRenderer.tsx (1)
12-13: Import paths already flagged in previous review.A previous review comment has already identified that the import paths for
ContentBlockandMessageRolemay be incorrect (should potentially be from@/features/session/typesinstead of@/shared/types). Please address the previous review comment.
🧹 Nitpick comments (20)
src/platform/tauri/commands/pty.ts (1)
9-28: Add explicit return type annotations for better type safety.The
ptyCommandsmethods correctly wrap Tauri invoke calls but lack explicit return type annotations. Since the platforminvokefunction returnsPromise<T>, these methods implicitly return promises, but making this explicit improves type safety and developer experience.Apply this diff to add return types:
export const ptyCommands = { spawn: (options: { id: string; command: string; args: string[]; cols: number; rows: number; cwd: string; - }) => + }): Promise<void> => invoke('spawn_pty', options), - write: (id: string, data: number[]) => + write: (id: string, data: number[]): Promise<void> => invoke('write_to_pty', { id, data }), - resize: (id: string, cols: number, rows: number) => + resize: (id: string, cols: number, rows: number): Promise<void> => invoke('resize_pty', { id, cols, rows }), - kill: (id: string) => + kill: (id: string): Promise<void> => invoke('kill_pty', { id }), };src/platform/tauri/invoke.ts (1)
18-39: Consider consistent fallback behavior across platform methods.The
invokefunction throws an error in non-Tauri environments (line 21), whilelistenreturns a no-op function (line 35). This inconsistency could lead to confusion—some calls fail loudly while others fail silently.Consider either:
- Making both throw errors for fail-fast behavior in development
- Making both return graceful fallbacks with clear logging
- Documenting the rationale for different strategies
Apply this pattern for consistency (option 1 - fail-fast):
export async function listen<T>(event: string, handler: (event: { payload: T }) => void): Promise<UnlistenFn> { if (!isTauriEnv) { console.warn(`[Platform] Tauri listen called in non-Tauri environment: ${event}`); - // Return noop unlisten function - return () => {}; + throw new Error(`Tauri events not available in web mode: ${event}`); } return tauriListen<T>(event, handler); }Or document the rationale if the difference is intentional (e.g., events are optional but commands are required).
src/shared/types/index.ts (1)
9-38: Consider adding deprecation tracking for temporary re-exports.The backward compatibility re-exports are a good migration strategy. However, the comments "to be removed later" lack specificity and could lead to these temporary exports becoming permanent, creating dual import paths indefinitely.
Consider:
- Adding TODO comments with issue tracking
- Documenting a deprecation timeline
- Using ESLint rules to flag these import paths in new code
Example:
-// Note: Workspace types moved to features/workspace/types.ts -// Re-export for backward compatibility (to be removed later) +// TODO(#14): Deprecated - Remove backward compatibility exports after all imports migrated +// Prefer: import { Workspace } from '@/features/workspace' +// @deprecated Use '@/features/workspace' instead export type { Workspace, WorkspaceState,src/features/settings/ui/sections/settings-sections/types.ts (1)
1-1: Consider using an alias instead of relative path for consistency.The relative path
../../../typesis fragile and inconsistent with the pattern used elsewhere in the refactor. Other files use aliases like@/shared/typesor@/features/*/typesfor better maintainability.Apply this diff to align with the established pattern:
-import type { Settings } from '../../../types'; +import type { Settings } from '@/features/settings/types';src/app/layouts/MainLayout.tsx (1)
61-628: Consider continued modularization of MainLayout.While the file has improved with WorkspaceHeader and FileChangesPanel extractions, MainLayout still orchestrates substantial business logic (~628 lines total):
- Workspace creation/archival handlers (lines 197-232)
- Repository cloning logic (lines 329-414)
- System prompt management (lines 256-279)
These could potentially be extracted into custom hooks (e.g.,
useWorkspaceActions,useRepositoryActions) to further reduce the layout's direct responsibilities. However, this is optional—the current structure is functional and improved from the previous state.src/platform/tauri/commands/socket.ts (1)
9-18: Add explicit return types and avoid duplication with SocketClient.
- Type the invokes to Promise to lock the contract.
- This duplicates logic in src/platform/tauri/socket/SocketClient.ts; consider having SocketClient delegate to this module (or vice versa) to prevent drift.
Apply:
export const socketCommands = { - connect: (path: string) => - invoke('socket_connect', { path }), + connect: (path: string): Promise<void> => + invoke<void>('socket_connect', { path }), - send: (data: string) => - invoke('socket_send', { data }), + send: (data: string): Promise<void> => + invoke<void>('socket_send', { data }), - close: () => - invoke('socket_close'), + close: (): Promise<void> => + invoke<void>('socket_close'), };src/features/session/ui/MessageItem.tsx (1)
95-97: Prefer stable keys over array index.Use a deterministic key to reduce remounts on re-order.
- contentBlocks.map((block: any, index: number) => ( - <BlockRenderer key={index} block={block} index={index} toolResultMap={toolResultMap} role={message.role} /> - )) + contentBlocks.map((block: any, index: number) => { + const key = block?.id ?? `${message.id}:${index}`; + return ( + <BlockRenderer key={key} block={block} index={index} toolResultMap={toolResultMap} role={message.role} /> + ); + })src/platform/tauri/socket/SocketClient.ts (1)
10-39: Type the API and delegate to socketCommands to avoid drift.
- Make return types explicit (Promise, Promise<() => void>).
- Reuse src/platform/tauri/commands/socket.ts to keep one source of truth.
- Optionally extract 'socket_message' into a shared constant.
-import { invoke, listen } from '../invoke'; +import { listen } from '../invoke'; +import { socketCommands } from '../commands/socket'; export class SocketClient { @@ - async connect(path: string): Promise<void> { - return invoke('socket_connect', { path }); - } + connect(path: string): Promise<void> { + return socketCommands.connect(path); + } @@ - async send(data: string): Promise<void> { - return invoke('socket_send', { data }); - } + send(data: string): Promise<void> { + return socketCommands.send(data); + } @@ - async close(): Promise<void> { - return invoke('socket_close'); - } + close(): Promise<void> { + return socketCommands.close(); + } @@ - onMessage(callback: (data: string) => void) { - return listen('socket_message', (event) => { + onMessage(callback: (data: string) => void): Promise<() => void> { + return listen('socket_message', (event) => { callback(event.payload as string); }); } }src/features/workspace/ui/FileChangesPanel.tsx (2)
67-71: Use stable keys for lists.Keys based on index can cause incorrect remounts; use unique identifiers.
- {devServers.map((server, index) => ( + {devServers.map((server) => ( <a - key={index} + key={server.url} href={server.url}- {fileChanges.map((file, index) => ( + {fileChanges.map((file) => ( <div - key={index} + key={file.file}Also applies to: 98-101
17-18: Remove unused loading state or render it.
loadingDiffis toggled but not shown. Either render a loading affordance or drop the state and its setters.-import { useState, useRef } from "react"; +import { useRef } from "react"; @@ - const [loadingDiff, setLoadingDiff] = useState(false); + // (Removed) loadingDiff state was unused in UI @@ - setLoadingDiff(true); @@ - } finally { - setLoadingDiff(false); - } + } finally { + // no-op + }Also applies to: 35-36, 54-55
src/features/session/ui/SessionPanel.tsx (2)
57-66: Debug logging is appropriately guarded for development only.The DEV-mode check ensures this logging won't appear in production. The logged information is useful for debugging session state.
Optional: Consider using a dedicated logger utility (e.g.,
@/shared/lib/logger) for consistency across the codebase, thoughconsole.logis acceptable for simple debug statements.
75-136: Good defensive programming with timestamp handling.The timestamp sanitization (lines 92-93, 110-121) properly guards against
NaNvalues, preventing runtime errors during rendering.Optional future enhancement: The
fileChangescomputation is complex and could be extracted into a custom hook (e.g.,useFileChangesFromMessages) for better separation of concerns and testability.src/features/browser/hooks/useBrowser.ts (2)
61-69: Treat null port as failure before setting running=trueIf get_browser_port returns null, UI that relies on a numeric port will misbehave. Fail fast and surface a clear error.
Apply:
- const port = await invoke<number | null>("get_browser_port"); + const port = await invoke<number | null>("get_browser_port"); const authToken = await invoke<string | null>("get_browser_auth_token"); + if (port == null) { + throw new Error("Browser server started but no port reported"); + } setStatus({ running: true, port, authToken, error: null, });
29-31: Optional: de‑dupe concurrent startServer callsRapid mounts/clicks can race two invokes. Consider an in‑flight guard.
Add near the hook top and wrap startServer:
-import { useState, useEffect, useCallback } from "react"; +import { useState, useEffect, useCallback, useRef } from "react"; @@ - const startServer = useCallback(async () => { + const inFlight = useRef<Promise<{ port: number | null; authToken: string | null }> | null>(null); + const startServer = useCallback(async () => { + if (inFlight.current) return inFlight.current; + const p = (async () => { try { if (isTauriMode()) { // ... } else { // ... } } catch (error) { // ... throw error; } - } + })(); + inFlight.current = p.finally(() => { inFlight.current = null; }); + return p; }, []);src/features/browser/ui/BrowserPanel.tsx (6)
176-188: Add Authorization header when authToken is availableProtect the injection endpoint by sending the token the backend exposes.
Apply:
- const response = await fetch(injectionUrl, { signal: controller.signal }); + const headers: HeadersInit = devBrowserStatus.authToken + ? { Authorization: `Bearer ${devBrowserStatus.authToken}` } + : {}; + const response = await fetch(injectionUrl, { signal: controller.signal, headers });
282-306: postMessage targetOrigin: handle file:// and include tabId for scopingFor file:// URLs, new URL(currentUrl).origin is "null", which can be brittle. Also include tabId in messages to scope actions.
Apply:
- let targetOrigin: string; + let targetOrigin: string; try { - targetOrigin = new URL(currentUrl).origin; + const o = new URL(currentUrl).origin; + targetOrigin = (o === 'null' ? '*' : o); } catch { addLog('warn', 'Invalid URL origin; cannot toggle element selector securely'); return; } @@ - iframeRef.current.contentWindow?.postMessage({ - type: 'disable-element-selection' - }, targetOrigin); + iframeRef.current.contentWindow?.postMessage({ + type: 'disable-element-selection', + tabId + }, targetOrigin); @@ - iframeRef.current.contentWindow?.postMessage({ - type: 'enable-element-selection' - }, targetOrigin); + iframeRef.current.contentWindow?.postMessage({ + type: 'enable-element-selection', + tabId + }, targetOrigin);Follow-up: ensure the injected script validates tabId.
406-437: Validate incoming messages with tabId and keep origin checkPrevents same-origin noise from other frames/tabs.
Apply:
- if (event.data.type === 'element-selected') { + if (event.data.tabId && event.data.tabId !== tabId) return; + if (event.data.type === 'element-selected') { handleElementSelected(event.data);
117-145: Optional: set loading state on back/forward for consistent UXShow spinner when navigating history, mirroring navigateToUrl().
Apply:
function goBack() { if (historyIndex > 0) { + setLoading(true); + setError(null); + setInjected(false); const newIndex = historyIndex - 1; @@ if (iframeRef.current) { iframeRef.current.src = previousUrl; } } } @@ function goForward() { if (historyIndex < history.length - 1) { + setLoading(true); + setError(null); + setInjected(false); const newIndex = historyIndex + 1; @@ if (iframeRef.current) { iframeRef.current.src = nextUrl; } } }
68-74: Optional: add devBrowserStatus.port to depsEnsures the log runs when port becomes available.
Apply:
- }, [devBrowserStatus.running]); + }, [devBrowserStatus.running, devBrowserStatus.port]);
369-376: Optional: escape backticks in Markdown payload to avoid formatting breaksPrevent accidental code-span termination.
Apply:
+ const escapeMd = (s: string) => s.replace(/`/g, '\\`'); @@ - const textContent = typeof el.innerText === 'string' && el.innerText - ? `**Text:** "${el.innerText.substring(0, 100)}${el.innerText.length > 100 ? '...' : ''}"` + const textContent = typeof el.innerText === 'string' && el.innerText + ? `**Text:** "${escapeMd(el.innerText.substring(0, 100))}${el.innerText.length > 100 ? '...' : ''}"` : ''; @@ - ? el.attributes.map((a: any) => `- **${a?.name ?? 'unknown'}**: \`"${a?.value ?? ''}"\``).join('\n') + ? el.attributes.map((a: any) => `- **${a?.name ?? 'unknown'}**: \`"${escapeMd(a?.value ?? '')}"\``).join('\n') : '_(No attributes)_';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (155)
BACKEND_CRASH_BUG.md(1 hunks)CODE_QUALITY_IMPLEMENTATION.md(1 hunks)CODE_QUALITY_IMPROVEMENTS.md(1 hunks)REFACTORING_BUGS_FOUND.md(1 hunks)REFACTORING_FINAL_REPORT.md(1 hunks)REFACTORING_PLAN_v2.md(1 hunks)REFACTORING_PROGRESS.md(1 hunks)REFACTORING_VERIFICATION.md(1 hunks)TESTING_REPORT.md(1 hunks)TEST_RESULTS.md(1 hunks)index.html(1 hunks)src/App.tsx(0 hunks)src/app/App.tsx(1 hunks)src/app/layouts/MainLayout.tsx(6 hunks)src/app/layouts/components/WorkspaceHeader.tsx(1 hunks)src/app/main.tsx(1 hunks)src/app/providers/QueryClientProvider.tsx(1 hunks)src/app/providers/index.ts(1 hunks)src/components/ui/EmptyState.tsx(2 hunks)src/components/ui/avatar.tsx(1 hunks)src/components/ui/badge.tsx(1 hunks)src/components/ui/button.tsx(1 hunks)src/components/ui/card.tsx(1 hunks)src/components/ui/checkbox.tsx(1 hunks)src/components/ui/dialog.tsx(1 hunks)src/components/ui/dropdown-menu.tsx(1 hunks)src/components/ui/input.tsx(1 hunks)src/components/ui/label.tsx(1 hunks)src/components/ui/scroll-area.tsx(1 hunks)src/components/ui/select.tsx(1 hunks)src/components/ui/separator.tsx(1 hunks)src/components/ui/sheet.tsx(1 hunks)src/components/ui/sidebar.tsx(1 hunks)src/components/ui/skeleton.tsx(1 hunks)src/components/ui/sonner.tsx(1 hunks)src/components/ui/tabs.tsx(1 hunks)src/components/ui/text-shimmer.tsx(1 hunks)src/components/ui/textarea.tsx(1 hunks)src/components/ui/tooltip.tsx(1 hunks)src/features/browser/components/index.ts(0 hunks)src/features/browser/hooks/index.ts(1 hunks)src/features/browser/hooks/useBrowser.ts(3 hunks)src/features/browser/index.ts(1 hunks)src/features/browser/ui/BrowserPanel.tsx(2 hunks)src/features/browser/ui/index.ts(1 hunks)src/features/repository/api/index.ts(1 hunks)src/features/repository/api/repository.queries.ts(1 hunks)src/features/repository/api/repository.service.ts(1 hunks)src/features/repository/index.ts(1 hunks)src/features/repository/ui/NewWorkspaceModal.tsx(1 hunks)src/features/repository/ui/RepoGroup.tsx(1 hunks)src/features/repository/ui/WelcomeView.tsx(1 hunks)src/features/repository/ui/WorkspaceItem.tsx(1 hunks)src/features/repository/ui/index.ts(1 hunks)src/features/session/api/index.ts(1 hunks)src/features/session/api/session.queries.ts(1 hunks)src/features/session/api/session.service.ts(1 hunks)src/features/session/hooks/index.ts(1 hunks)src/features/session/hooks/useAutoScroll.ts(1 hunks)src/features/session/index.ts(1 hunks)src/features/session/types.ts(2 hunks)src/features/session/ui/Chat.tsx(1 hunks)src/features/session/ui/MessageInput.tsx(1 hunks)src/features/session/ui/MessageItem.tsx(2 hunks)src/features/session/ui/SessionPanel.tsx(4 hunks)src/features/session/ui/blocks/BlockRenderer.tsx(1 hunks)src/features/session/ui/blocks/TextBlock.tsx(1 hunks)src/features/session/ui/blocks/ThinkingBlock.tsx(1 hunks)src/features/session/ui/blocks/ToolResultBlock.tsx(1 hunks)src/features/session/ui/blocks/ToolUseBlock.tsx(1 hunks)src/features/session/ui/chat-types.ts(1 hunks)src/features/session/ui/index.ts(1 hunks)src/features/session/ui/tools/ToolRegistry.tsx(1 hunks)src/features/session/ui/tools/components/CodeBlock.tsx(1 hunks)src/features/session/ui/tools/components/CopyButton.tsx(1 hunks)src/features/session/ui/tools/components/FilePathDisplay.tsx(2 hunks)src/features/session/ui/tools/components/SyntaxHighlighter.tsx(1 hunks)src/features/session/ui/tools/renderers/BashOutputToolRenderer.tsx(1 hunks)src/features/session/ui/tools/renderers/BashToolRenderer.tsx(3 hunks)src/features/session/ui/tools/renderers/DefaultToolRenderer.tsx(3 hunks)src/features/session/ui/tools/renderers/EditToolRenderer.tsx(1 hunks)src/features/session/ui/tools/renderers/GlobToolRenderer.tsx(1 hunks)src/features/session/ui/tools/renderers/GrepToolRenderer.tsx(1 hunks)src/features/session/ui/tools/renderers/KillShellToolRenderer.tsx(2 hunks)src/features/session/ui/tools/renderers/LSToolRenderer.tsx(1 hunks)src/features/session/ui/tools/renderers/MultiEditToolRenderer.tsx(1 hunks)src/features/session/ui/tools/renderers/ReadToolRenderer.tsx(1 hunks)src/features/session/ui/tools/renderers/TaskToolRenderer.tsx(1 hunks)src/features/session/ui/tools/renderers/TodoWriteToolRenderer.tsx(3 hunks)src/features/session/ui/tools/renderers/WebFetchToolRenderer.tsx(1 hunks)src/features/session/ui/tools/renderers/WebSearchToolRenderer.tsx(4 hunks)src/features/session/ui/tools/renderers/WriteToolRenderer.tsx(1 hunks)src/features/settings/api/index.ts(1 hunks)src/features/settings/api/memory.service.ts(1 hunks)src/features/settings/api/settings.queries.ts(2 hunks)src/features/settings/api/settings.service.ts(2 hunks)src/features/settings/index.ts(1 hunks)src/features/settings/ui/SettingsModal.tsx(1 hunks)src/features/settings/ui/index.ts(1 hunks)src/features/settings/ui/sections/index.ts(1 hunks)src/features/settings/ui/sections/settings-sections/MemorySection.tsx(1 hunks)src/features/settings/ui/sections/settings-sections/types.ts(1 hunks)src/features/sidebar/index.ts(1 hunks)src/features/sidebar/store/index.ts(1 hunks)src/features/sidebar/store/sidebarStore.ts(1 hunks)src/features/sidebar/ui/AppSidebar.tsx(2 hunks)src/features/sidebar/ui/index.ts(1 hunks)src/features/terminal/index.ts(1 hunks)src/features/terminal/ui/Terminal.tsx(6 hunks)src/features/terminal/ui/index.ts(1 hunks)src/features/workspace/api/index.ts(1 hunks)src/features/workspace/api/workspace.queries.ts(2 hunks)src/features/workspace/api/workspace.service.ts(1 hunks)src/features/workspace/components/FileChangesPanel.tsx(0 hunks)src/features/workspace/components/chat/index.ts(0 hunks)src/features/workspace/components/chat/message/MessageItem.tsx(0 hunks)src/features/workspace/components/chat/message/index.ts(0 hunks)src/features/workspace/components/index.ts(0 hunks)src/features/workspace/index.ts(1 hunks)src/features/workspace/store/index.ts(1 hunks)src/features/workspace/store/workspaceStore.ts(1 hunks)src/features/workspace/types.ts(1 hunks)src/features/workspace/ui/FileChangesPanel.tsx(1 hunks)src/features/workspace/ui/index.ts(1 hunks)src/hooks/index.ts(0 hunks)src/hooks/queries/index.ts(0 hunks)src/hooks/useDashboardData.ts(0 hunks)src/hooks/useDiffStats.ts(0 hunks)src/hooks/useFileChanges.ts(0 hunks)src/hooks/useMessages.ts(0 hunks)src/hooks/useWorkspaces.ts(0 hunks)src/platform/index.ts(1 hunks)src/platform/socket/index.ts(1 hunks)src/platform/socket/socketService.ts(4 hunks)src/platform/tauri/commands/index.ts(1 hunks)src/platform/tauri/commands/pty.ts(1 hunks)src/platform/tauri/commands/socket.ts(1 hunks)src/platform/tauri/index.ts(1 hunks)src/platform/tauri/invoke.ts(1 hunks)src/platform/tauri/socket/SocketClient.ts(1 hunks)src/services/index.ts(0 hunks)src/shared/api/index.ts(1 hunks)src/shared/components/EmptyState.tsx(1 hunks)src/shared/components/index.ts(1 hunks)src/shared/config/index.ts(1 hunks)src/shared/hooks/index.ts(1 hunks)src/shared/hooks/useKeyboardShortcuts.ts(1 hunks)src/shared/hooks/useSocket.ts(1 hunks)src/shared/lib/index.ts(1 hunks)src/shared/stores/uiStore.ts(0 hunks)src/shared/types/index.ts(2 hunks)src/stores/index.ts(0 hunks)src/utils/index.ts(0 hunks)tsconfig.json(1 hunks)vite.config.ts(1 hunks)
💤 Files with no reviewable changes (18)
- src/features/browser/components/index.ts
- src/features/workspace/components/chat/message/MessageItem.tsx
- src/hooks/useFileChanges.ts
- src/hooks/useMessages.ts
- src/shared/stores/uiStore.ts
- src/App.tsx
- src/hooks/index.ts
- src/stores/index.ts
- src/features/workspace/components/chat/message/index.ts
- src/features/workspace/components/index.ts
- src/hooks/useDashboardData.ts
- src/hooks/queries/index.ts
- src/features/workspace/components/chat/index.ts
- src/features/workspace/components/FileChangesPanel.tsx
- src/hooks/useDiffStats.ts
- src/services/index.ts
- src/utils/index.ts
- src/hooks/useWorkspaces.ts
🧰 Additional context used
🧬 Code graph analysis (16)
src/features/sidebar/ui/AppSidebar.tsx (2)
src/shared/stores/uiStore.ts (1)
useUIStore(33-116)src/features/sidebar/store/sidebarStore.ts (1)
useSidebarStore(9-21)
src/app/layouts/components/WorkspaceHeader.tsx (1)
backend/server.cjs (1)
branch(95-95)
src/app/App.tsx (1)
src/components/ui/sonner.tsx (1)
Toaster(29-29)
src/app/providers/QueryClientProvider.tsx (2)
src/app/providers/index.ts (1)
QueryClientProvider(1-1)src/shared/api/queryClient.ts (1)
queryClient(8-39)
src/features/terminal/ui/Terminal.tsx (1)
src/platform/tauri/commands/pty.ts (1)
ptyCommands(9-28)
src/platform/tauri/commands/pty.ts (2)
src/platform/tauri/invoke.ts (1)
invoke(18-25)src/platform/tauri/index.ts (1)
invoke(6-6)
src/features/browser/hooks/useBrowser.ts (3)
src/features/browser/hooks/index.ts (1)
useBrowser(1-1)src/features/browser/index.ts (1)
useBrowser(2-2)backend/lib/message-sanitizer.cjs (1)
errorMessage(118-118)
src/features/session/ui/SessionPanel.tsx (2)
src/features/session/index.ts (2)
SessionPanelRef(2-2)SessionPanel(1-1)src/features/session/ui/index.ts (2)
SessionPanelRef(2-2)SessionPanel(1-1)
src/features/browser/ui/BrowserPanel.tsx (3)
src/features/browser/hooks/index.ts (1)
useBrowser(1-1)src/features/browser/hooks/useBrowser.ts (1)
useBrowser(21-203)src/features/browser/index.ts (1)
useBrowser(2-2)
src/platform/tauri/commands/socket.ts (2)
src/platform/tauri/invoke.ts (1)
invoke(18-25)src/platform/tauri/index.ts (1)
invoke(6-6)
src/features/settings/api/settings.service.ts (1)
src/shared/api/client.ts (1)
apiClient(131-131)
src/features/sidebar/store/sidebarStore.ts (2)
src/features/sidebar/index.ts (1)
useSidebarStore(3-3)src/features/sidebar/store/index.ts (1)
useSidebarStore(1-1)
src/platform/tauri/invoke.ts (1)
src/platform/tauri/index.ts (3)
isTauriEnv(6-6)invoke(6-6)isTauriAvailable(6-6)
src/app/layouts/MainLayout.tsx (4)
src/features/session/index.ts (2)
SessionPanelRef(2-2)SessionPanel(1-1)src/features/session/ui/index.ts (2)
SessionPanelRef(2-2)SessionPanel(1-1)src/app/layouts/components/WorkspaceHeader.tsx (1)
WorkspaceHeader(11-24)src/features/workspace/ui/FileChangesPanel.tsx (1)
FileChangesPanel(16-139)
src/features/workspace/api/workspace.queries.ts (3)
src/features/workspace/types.ts (1)
DiffStats(46-49)src/shared/types/index.ts (1)
DiffStats(15-15)src/shared/api/queryKeys.ts (1)
queryKeys(12-51)
src/features/workspace/ui/FileChangesPanel.tsx (4)
src/features/workspace/types.ts (1)
Workspace(14-29)src/shared/stores/uiStore.ts (1)
useUIStore(33-116)src/features/workspace/api/workspace.queries.ts (2)
useFileChanges(126-136)useDevServers(153-163)src/features/workspace/api/workspace.service.ts (1)
WorkspaceService(20-115)
| # 🐛 REFACTORING BUGS FOUND | ||
|
|
||
| **Date:** 2025-10-21 | ||
| **Status:** ❌ CRITICAL ISSUES - App Not Functional | ||
| **Context:** Post-refactoring verification found multiple critical runtime bugs | ||
|
|
There was a problem hiding this comment.
Conflicting status between bug report and test results documentation.
This document reports "❌ CRITICAL ISSUES - App Not Functional" (line 4), while TEST_RESULTS.md (also dated October 21, 2025) states "✅ Application is fully functional in web development mode." Both documents appear to be from the same date but report contradictory application states.
This inconsistency suggests either:
- The bugs were found and then fixed (in which case this document should be updated with resolution status)
- The test results missed the critical issues described here
- The documents represent different test scenarios or environments
Please clarify the current application state and update the documentation accordingly. If the bugs described here have been resolved, add a resolution section or archive this document.
🤖 Prompt for AI Agents
In REFACTORING_BUGS_FOUND.md around lines 1 to 6, the status line "❌ CRITICAL
ISSUES - App Not Functional" conflicts with TEST_RESULTS.md (same date) which
reports the app as fully functional; update documentation to resolve the
inconsistency by verifying current app state, then either (A) if bugs are fixed:
change the status to a resolved/archived state, add a short "Resolution" section
summarizing fixes, link to the passing TEST_RESULTS.md and include the
resolution date and committer, or (B) if bugs persist: update TEST_RESULTS.md to
reflect failures and add reproducible steps and failing test identifiers here;
ensure both documents include environment and date metadata and cross-link each
other for clarity.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
src/features/session/ui/MessageItem.tsx (1)
52-68: Consider reducing development console noise.The documentation acknowledges that "thousands of
[MessageItem] Skipping empty messagelogs" may appear in development (lines 37-38). While the filtering logic is correct and expected, this level of console output could make debugging difficult.Consider one of these approaches:
- Remove the log entirely (the filtering is documented and expected)
- Add a debug flag:
if (import.meta.env.DEV && import.meta.env.VITE_DEBUG_MESSAGES)- Throttle or batch these logs to reduce frequency
src/features/browser/hooks/useBrowser.ts (4)
4-9: Rename the status type for consistency with the hook name.DevBrowserStatus reads like legacy naming after the useBrowser rename. Prefer BrowserStatus to avoid drift.
-interface DevBrowserStatus { +interface BrowserStatus { running: boolean; port: number | null; authToken: string | null; error: string | null; } @@ -export function useBrowser() { - const [status, setStatus] = useState<DevBrowserStatus>({ +export function useBrowser() { + const [status, setStatus] = useState<BrowserStatus>({
83-85: Avoid unused body parse; a HEAD or status-only check is enough.You don’t use the JSON body; remove the parse, optionally switch to HEAD.
- if (response.ok) { - await response.json(); // Response checked; details unused + if (response.ok) { setStatus({Optionally:
- response = await fetch('http://localhost:3000/health', { signal: controller.signal }); + response = await fetch('http://localhost:3000/health', { method: 'HEAD', signal: controller.signal });
114-129: Standardize stop logging to safe summary + naming; align with “[useBrowser]”.Keep logs consistent and avoid dumping full Error objects.
- // Stop dev-browser server + // Stop browser server @@ - } catch (error) { - console.error("Failed to stop dev-browser:", error); + } catch (error) { + const kind = error instanceof Error ? error.name : typeof error; + const msg = error instanceof Error ? error.message : String(error); + console.error('[useBrowser] Error stopping server:', kind, msg);
73-96: Deduplicate health URL and port; define a single constant.Prevents drift and eases future changes.
@@ -const isTauriMode = () => { +const isTauriMode = () => { try { @@ } }; + +const WEB_HEALTH_URL = 'http://localhost:3000/health'; @@ - response = await fetch('http://localhost:3000/health', { signal: controller.signal }); + response = await fetch(WEB_HEALTH_URL, { signal: controller.signal }); @@ - response = await fetch('http://localhost:3000/health', { signal: controller.signal }); + response = await fetch(WEB_HEALTH_URL, { signal: controller.signal });Also applies to: 158-191
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (9)
src/app/App.tsx(1 hunks)src/app/layouts/components/WorkspaceHeader.tsx(1 hunks)src/features/browser/hooks/useBrowser.ts(4 hunks)src/features/session/ui/MessageItem.tsx(1 hunks)src/features/terminal/ui/Terminal.tsx(4 hunks)src/features/workspace/ui/FileChangesPanel.tsx(1 hunks)src/platform/tauri/commands/pty.ts(1 hunks)src/platform/tauri/commands/socket.ts(1 hunks)src/platform/tauri/socket/SocketClient.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- src/app/App.tsx
- src/platform/tauri/commands/socket.ts
- src/features/workspace/ui/FileChangesPanel.tsx
- src/platform/tauri/socket/SocketClient.ts
- src/features/terminal/ui/Terminal.tsx
- src/platform/tauri/commands/pty.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{tsx,jsx,css}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{tsx,jsx,css}: Avoid hardcoding colors; always use color tokens from Tailwind config
Always use font tokens from the Tailwind config (avoid hardcoded font sizes/families)
Use consistent paddings with a default of 16px unless there’s a strong reason otherwise
Files:
src/features/session/ui/MessageItem.tsxsrc/app/layouts/components/WorkspaceHeader.tsx
**/*.{css,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{css,tsx,jsx}: Default to ease-out for most animations
Animations should be shorter than 1s; most between 0.2s and 0.3s
Do not use built-in CSS easings excepteaseorlinear
Prefer ease-out for entering/user-initiated interactions; use ease-in-out for in-place movements; generally avoid ease-in
Useeasewith 200ms duration for simple hover transitions (color, background-color, opacity)
Disable hover transitions on touch devices using@media (hover: hover) and (pointer: fine)
Respect prefers-reduced-motion: disable transform-based animations when reduced motion is requested
Animate elements from their trigger by setting an appropriate transform-origin
Prefer animating opacity and transform; avoid animating positional properties like top/left
Do not animate drag gestures using CSS variables
Do not animate blur values higher than 20px
Usewill-changesparingly and only for transform, opacity, clipPath, or filter
Files:
src/features/session/ui/MessageItem.tsxsrc/app/layouts/components/WorkspaceHeader.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{tsx,jsx}: When using Framer Motion, use transform rather than x/y for hardware-accelerated animations
Default to spring animations with Framer Motion
Avoid bouncy spring animations unless working with drag gestures
Files:
src/features/session/ui/MessageItem.tsxsrc/app/layouts/components/WorkspaceHeader.tsx
🧬 Code graph analysis (1)
src/features/browser/hooks/useBrowser.ts (3)
src/features/browser/hooks/index.ts (1)
useBrowser(1-1)src/features/browser/index.ts (1)
useBrowser(2-2)backend/lib/message-sanitizer.cjs (1)
errorMessage(118-118)
🔇 Additional comments (5)
src/app/layouts/components/WorkspaceHeader.tsx (1)
1-23: LGTM! Clean component implementation following FSD-Lite refactor patterns.The WorkspaceHeader component is well-structured with proper type safety, uses Tailwind color tokens throughout (avoiding hardcoded colors), and maintains consistent padding (16px horizontal per guidelines). The compact vertical padding (12px) is appropriate for a header. The flex layout clearly separates the left controls (sidebar trigger, separator, branch name) from the right dropdown.
src/features/session/ui/MessageItem.tsx (1)
27-68: Documentation helps explain complex filtering logic.The extensive documentation (25 lines) explaining the message filtering behavior is helpful for developers who may be confused by skipped messages. This level of documentation suggests the filtering pattern might be non-intuitive, but it appears to be necessary given Claude's message format (tool_result blocks must be linked to tool_use blocks via the toolResultMap).
The documentation includes clear troubleshooting steps and architectural explanation, which should help prevent confusion. Well done on the thoroughness.
src/features/browser/hooks/useBrowser.ts (3)
74-81: Nice: timeout + AbortController with guaranteed cleanup.Good use of AbortController and try/finally to clear the timer. LGTM.
99-104: Error logging hardening looks good.Replaces risky JSON.stringify with safe kind/message summary; consistent prefix. LGTM.
35-41: Verify dev-browser path resolution in packaged Tauri builds.Relative paths in Tauri's resource API resolve to
$RESOURCE/with directory structure preserved, yet the dev-browser path is passed directly from the frontend without using this API. Tauri replaces..path components, and you must use PathResolver to resolve resources correctly in packaged builds.The issue: the fallback path
"../../../dev-browser"will fail in packaged builds because it resolves relative to the current working directory, not from the app bundle. Unlike the backend (which usesapp.path().resource_dir()), the dev-browser command receives a raw path string and only validates it withpath.exists(). The dev-browser binary is not listed intauri.conf.jsonbundle resources, and noPathResolveris used insrc-tauri/src/commands.rs.Ensure VITE_DEV_BROWSER_PATH is set to an absolute path in
.envfor production builds, or useapp.path().resource_dir()in the Rust command to resolve the binary location robustly across dev and packaged modes.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/features/browser/hooks/useBrowser.ts (1)
55-56: Consider extracting retry constants.The hardcoded retry values (10 attempts, 500ms delay) could be extracted to named constants at the module level for easier tuning and self-documentation.
+const MAX_STARTUP_ATTEMPTS = 10; +const STARTUP_RETRY_DELAY_MS = 500; + export function useBrowser() { // ... - const maxAttempts = 10; - const delayMs = 500; + const maxAttempts = MAX_STARTUP_ATTEMPTS; + const delayMs = STARTUP_RETRY_DELAY_MS;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (2)
src/features/browser/hooks/useBrowser.ts(8 hunks)src/features/session/ui/MessageItem.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{tsx,jsx,css}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{tsx,jsx,css}: Avoid hardcoding colors; always use color tokens from Tailwind config
Always use font tokens from the Tailwind config (avoid hardcoded font sizes/families)
Use consistent paddings with a default of 16px unless there’s a strong reason otherwise
Files:
src/features/session/ui/MessageItem.tsx
**/*.{css,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{css,tsx,jsx}: Default to ease-out for most animations
Animations should be shorter than 1s; most between 0.2s and 0.3s
Do not use built-in CSS easings excepteaseorlinear
Prefer ease-out for entering/user-initiated interactions; use ease-in-out for in-place movements; generally avoid ease-in
Useeasewith 200ms duration for simple hover transitions (color, background-color, opacity)
Disable hover transitions on touch devices using@media (hover: hover) and (pointer: fine)
Respect prefers-reduced-motion: disable transform-based animations when reduced motion is requested
Animate elements from their trigger by setting an appropriate transform-origin
Prefer animating opacity and transform; avoid animating positional properties like top/left
Do not animate drag gestures using CSS variables
Do not animate blur values higher than 20px
Usewill-changesparingly and only for transform, opacity, clipPath, or filter
Files:
src/features/session/ui/MessageItem.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{tsx,jsx}: When using Framer Motion, use transform rather than x/y for hardware-accelerated animations
Default to spring animations with Framer Motion
Avoid bouncy spring animations unless working with drag gestures
Files:
src/features/session/ui/MessageItem.tsx
🧠 Learnings (2)
📚 Learning: 2025-10-21T08:46:30.798Z
Learnt from: CR
PR: zvadaadam/box-ide#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-21T08:46:30.798Z
Learning: Applies to **/*.{tsx,jsx,css} : Always use font tokens from the Tailwind config (avoid hardcoded font sizes/families)
Applied to files:
src/features/session/ui/MessageItem.tsx
📚 Learning: 2025-10-21T08:46:30.798Z
Learnt from: CR
PR: zvadaadam/box-ide#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-21T08:46:30.798Z
Learning: Applies to **/*.{tsx,jsx,css} : Avoid hardcoding colors; always use color tokens from Tailwind config
Applied to files:
src/features/session/ui/MessageItem.tsx
🧬 Code graph analysis (1)
src/features/browser/hooks/useBrowser.ts (3)
src/features/browser/hooks/index.ts (1)
useBrowser(1-1)src/features/browser/index.ts (1)
useBrowser(2-2)backend/lib/message-sanitizer.cjs (1)
errorMessage(118-118)
🔇 Additional comments (6)
src/features/browser/hooks/useBrowser.ts (4)
21-21: Good refactor: health URL extracted to constant.Extracting the health check URL to a named constant improves maintainability compared to inline strings.
40-52: LGTM: Path logging properly secured.The dev-only guard and basename extraction correctly address the previous concern about leaking filesystem paths. The logging is now safe and only active in development mode.
107-111: LGTM: Error logging properly hardened.The safe error extraction correctly addresses the previous concern about JSON.stringify throwing on circular structures. The logging now uses minimal, safe summaries without risk of leaking internals or throwing.
136-138: LGTM: Consistent safe error logging.Error handling in
stopServerfollows the same safe pattern asstartServer, ensuring consistent and secure error logging throughout the hook.src/features/session/ui/MessageItem.tsx (2)
18-24: Good improvement to type safety!The
ParsedContenttype and its usage inMessageItemPropsproperly addresses the previous feedback about theparseContentfunction returningany. This makes the component more type-safe and easier to maintain.
16-16: Side-effect import is safe—verification confirms idempotency.The tool registry implementation includes an idempotency guard (
__didRegisterToolsflag) that prevents double registration even if the module is imported multiple times. Registration is intentionally called at module load time and is designed to be safe for repeated imports, including during hot module replacement in development.
| const controller = new AbortController(); | ||
| const timeoutId = setTimeout(() => controller.abort(), 3000); | ||
| let response: Response; | ||
| try { | ||
| response = await fetch(WEB_HEALTH_URL, { signal: controller.signal }); | ||
| } finally { | ||
| clearTimeout(timeoutId); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify or document the timeout difference.
The AbortController implementation is correct, but startServer uses a 3000ms timeout while checkStatus (line 171) uses 2000ms. If intentional (e.g., startup needs more time), consider adding a comment. Otherwise, use a shared constant for consistency.
🏁 Script executed:
#!/bin/bash
# Search for other timeout values in browser-related code
rg -nP '\bsetTimeout\b.*\d+' src/features/browser/Length of output: 344
🏁 Script executed:
#!/bin/bash
# Search for timeout-related constants or configuration
rg -nP '(const|export).*\b(TIMEOUT|timeout)\b' src/features/browser/ -A2 -B2Length of output: 0
🏁 Script executed:
#!/bin/bash
# Get context around each setTimeout to understand the operation
echo "=== BrowserPanel.tsx:185 ===" && sed -n '180,190p' src/features/browser/ui/BrowserPanel.tsx
echo -e "\n=== useBrowser.ts:85 ===" && sed -n '80,95p' src/features/browser/hooks/useBrowser.ts
echo -e "\n=== useBrowser.ts:171 ===" && sed -n '165,180p' src/features/browser/hooks/useBrowser.tsLength of output: 1727
🏁 Script executed:
#!/bin/bash
# Check function/method names around each setTimeout
echo "=== BrowserPanel.tsx context ===" && sed -n '170,195p' src/features/browser/ui/BrowserPanel.tsx
echo -e "\n=== useBrowser.ts:85 context ===" && sed -n '70,100p' src/features/browser/hooks/useBrowser.ts
echo -e "\n=== useBrowser.ts:171 context ===" && sed -n '160,185p' src/features/browser/hooks/useBrowser.tsLength of output: 2863
🏁 Script executed:
#!/bin/bash
# Check if there are any comments explaining the timeout choices
rg -nP '\b(3000|2000|10000)\b' src/features/browser/ -B3 -A1 | grep -E '(//|/\*|timeout|abort|3000|2000|10000)'Length of output: 1413
Extract timeout values into named constants and document the differences.
The codebase has three inconsistent timeout values (10000ms in BrowserPanel.tsx:185, 3000ms in useBrowser.ts:85, and 2000ms in useBrowser.ts:171) with no justification. While the durations may have context-specific reasons (startup vs. status checks), they should be extracted into named constants with comments explaining each choice. This improves maintainability and prevents accidental reuse of the wrong timeout elsewhere.
🤖 Prompt for AI Agents
In src/features/browser/hooks/useBrowser.ts around lines 84 to 91, the inline
3000ms timeout should be extracted into a named constant (e.g.,
HEALTHCHECK_TIMEOUT_MS) and documented; similarly locate the other two
hard-coded timeouts (10000ms in src/features/browser/BrowserPanel.tsx:185 and
2000ms in src/features/browser/hooks/useBrowser.ts:171), extract them into
clearly named constants (e.g., STARTUP_TIMEOUT_MS for the 10000ms and
STATUS_POLL_TIMEOUT_MS for the 2000ms) placed in a shared module or the top of
the feature file, add brief comments explaining why each duration differs
(startup vs health check vs status poll), and replace the numeric literals with
these constants to avoid accidental reuse and improve maintainability.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/features/browser/hooks/useBrowser.ts (1)
21-31: Well done addressing previous feedback; consider extracting the port constant.The timeout constants and documentation effectively address the earlier review concern about hardcoded timeouts. The distinction between STARTUP_TIMEOUT_MS and STATUS_CHECK_TIMEOUT_MS is now clear.
However, the port
3000is hardcoded in multiple locations (lines 21, 106, 111, 191, 208). Extracting it to a named constant would improve maintainability if the port ever needs to change.Consider applying this diff:
const WEB_HEALTH_URL = 'http://localhost:3000/health'; +const WEB_MODE_PORT = 3000; +const WEB_HEALTH_URL = `http://localhost:${WEB_MODE_PORT}/health`;Then replace the hardcoded
3000literals on lines 106, 111, 191, and update line 208 to reference the constant in the error message.src/features/session/ui/MessageItem.tsx (1)
74-75: Remove unused variables.The
isUserandisAssistantvariables are defined but never used. The role styling is determined directly by the ternary on line 77.Apply this diff:
- const isUser = message.role === 'user'; - const isAssistant = message.role === 'assistant'; - const roleStyles = isUser ? chatTheme.message.user : chatTheme.message.assistant;Or simplify to:
- const isUser = message.role === 'user'; - const isAssistant = message.role === 'assistant'; - - const roleStyles = isUser + const roleStyles = message.role === 'user' ? chatTheme.message.user : chatTheme.message.assistant;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (3)
src/features/browser/hooks/useBrowser.ts(8 hunks)src/features/browser/ui/BrowserPanel.tsx(3 hunks)src/features/session/ui/MessageItem.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/features/browser/ui/BrowserPanel.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{tsx,jsx,css}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{tsx,jsx,css}: Avoid hardcoding colors; always use color tokens from Tailwind config
Always use font tokens from the Tailwind config (avoid hardcoded font sizes/families)
Use consistent paddings with a default of 16px unless there’s a strong reason otherwise
Files:
src/features/session/ui/MessageItem.tsx
**/*.{css,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{css,tsx,jsx}: Default to ease-out for most animations
Animations should be shorter than 1s; most between 0.2s and 0.3s
Do not use built-in CSS easings excepteaseorlinear
Prefer ease-out for entering/user-initiated interactions; use ease-in-out for in-place movements; generally avoid ease-in
Useeasewith 200ms duration for simple hover transitions (color, background-color, opacity)
Disable hover transitions on touch devices using@media (hover: hover) and (pointer: fine)
Respect prefers-reduced-motion: disable transform-based animations when reduced motion is requested
Animate elements from their trigger by setting an appropriate transform-origin
Prefer animating opacity and transform; avoid animating positional properties like top/left
Do not animate drag gestures using CSS variables
Do not animate blur values higher than 20px
Usewill-changesparingly and only for transform, opacity, clipPath, or filter
Files:
src/features/session/ui/MessageItem.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{tsx,jsx}: When using Framer Motion, use transform rather than x/y for hardware-accelerated animations
Default to spring animations with Framer Motion
Avoid bouncy spring animations unless working with drag gestures
Files:
src/features/session/ui/MessageItem.tsx
🧠 Learnings (1)
📚 Learning: 2025-10-21T08:46:30.798Z
Learnt from: CR
PR: zvadaadam/box-ide#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-21T08:46:30.798Z
Learning: Applies to **/*.{tsx,jsx,css} : Always use font tokens from the Tailwind config (avoid hardcoded font sizes/families)
Applied to files:
src/features/session/ui/MessageItem.tsx
🧬 Code graph analysis (2)
src/features/session/ui/MessageItem.tsx (7)
src/features/session/types.ts (2)
ContentBlock(26-26)Message(13-18)src/shared/types/index.ts (2)
ContentBlock(27-27)Message(24-24)src/features/session/ui/chat-types.ts (1)
ToolResultMap(13-13)src/features/session/ui/theme/chatTheme.ts (1)
chatTheme(8-129)src/features/session/ui/theme/index.ts (1)
chatTheme(1-1)src/shared/lib/utils.ts (1)
cn(4-6)src/features/session/ui/blocks/BlockRenderer.tsx (1)
BlockRenderer(25-67)
src/features/browser/hooks/useBrowser.ts (3)
src/features/browser/hooks/index.ts (1)
useBrowser(1-1)src/features/browser/index.ts (1)
useBrowser(2-2)backend/lib/message-sanitizer.cjs (1)
errorMessage(118-118)
🔇 Additional comments (6)
src/features/browser/hooks/useBrowser.ts (5)
50-62: Excellent fix for the path logging security concern.The DEV environment guard combined with basename extraction properly addresses the previous issue about leaking filesystem paths. The cross-platform path splitting is correct.
94-101: LGTM! Correct AbortController timeout pattern.The AbortController implementation properly enforces the startup timeout, and the
finallyblock ensures cleanup regardless of success or failure. UsingSTARTUP_TIMEOUT_MShere is appropriate for the cold-start context.
117-121: Great improvement to error logging.The error handling now safely extracts
kindandmsgwithout attempting to serialize arbitrary objects. This eliminates the risk of circular reference exceptions and inadvertent leakage of sensitive data, directly addressing the previous major concern.
146-148: LGTM! Consistent error handling.The error handling in
stopServerfollows the same safe pattern asstartServer, maintaining consistency across the hook.
180-187: LGTM! Appropriate timeout for status checks.The
STATUS_CHECK_TIMEOUT_MS(2000ms) is correctly applied here for periodic health checks, providing faster feedback than the startup timeout. The AbortController cleanup is correct.src/features/session/ui/MessageItem.tsx (1)
104-110: Proper runtime handling of mixed content blocks.The code correctly handles the
ContentBlock | stringunion with a type guard on line 106, ensuring only valid blocks reachBlockRenderer. This is the correct runtime behavior.Note: Once the
ParsedContenttype on line 18 is updated to(ContentBlock | string)[] | string | null, TypeScript will properly validate this code.
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 (1)
src/features/browser/hooks/useBrowser.ts (1)
81-92: Don’t mark running until a non-null port is available. Add a short retry.Avoid a state where running=true but port=null. Retry briefly, then fail fast.
-// Get port and auth token -const port = await invoke<number | null>("get_browser_port"); -const authToken = await invoke<string | null>("get_browser_auth_token"); +// Get port and auth token (retry briefly if port isn't published yet) +let port = await invoke<number | null>("get_browser_port"); +for (let i = 0; i < 5 && (port == null); i++) { + await new Promise((r) => setTimeout(r, 200)); + port = await invoke<number | null>("get_browser_port"); +} +if (port == null) { + throw new Error("Browser started but no port reported"); +} +const authToken = await invoke<string | null>("get_browser_auth_token");
🧹 Nitpick comments (7)
src/features/browser/hooks/useBrowser.ts (7)
4-9: Consider exporting the BrowserStatus type (or move to shared/types).If other modules consume this shape, make it explicit and reusable. Otherwise, keep it internal.
-interface BrowserStatus { +export interface BrowserStatus { running: boolean; port: number | null; authToken: string | null; error: string | null; }
11-19: Deduplicate Tauri detection via a shared platform util.If you already have a platform abstraction, reuse it here to avoid drift across files.
21-23: Make host/port configurable and derive protocol; avoid hard-coded localhost/http.Support custom host/port via env, and use the current protocol to prevent mixed-content issues in HTTPS.
-const WEB_MODE_PORT = 3000; -const WEB_HEALTH_URL = `http://localhost:${WEB_MODE_PORT}/health`; +// Configurable web-mode host/port with sensible defaults +const WEB_MODE_PORT = + Number(import.meta.env.VITE_BROWSER_WEB_PORT ?? 3000); +const WEB_MODE_HOST = + import.meta.env.VITE_BROWSER_WEB_HOST ?? 'localhost'; +const WEB_HEALTH_URL = + `${window.location.protocol}//${WEB_MODE_HOST}:${WEB_MODE_PORT}/health`;Also applies to: 104-113, 192-193, 209-209
24-33: Timeout constants: good extraction. Consider centralizing across the feature.If BrowserPanel or other browser files use timeouts, lift these to a feature-level constants module to keep them consistent.
65-75: Harden the startup poll loop against transient invoke errors.If invoke("is_browser_running") throws while the backend initializes, the loop exits early. Catch and continue.
-while (attempt < maxAttempts && !serverReady) { - await new Promise(resolve => setTimeout(resolve, delayMs)); - serverReady = await invoke<boolean>("is_browser_running"); - attempt++; -} +while (attempt < maxAttempts && !serverReady) { + await new Promise((resolve) => setTimeout(resolve, delayMs)); + try { + serverReady = await invoke<boolean>("is_browser_running"); + } catch { + // Backend may not be ready yet; keep retrying + serverReady = false; + } + attempt++; +}
95-102: Health checks: disable caching to avoid false positives.Send cache: 'no-store' on health pings to prevent cached responses.
-response = await fetch(WEB_HEALTH_URL, { signal: controller.signal }); +response = await fetch(WEB_HEALTH_URL, { signal: controller.signal, cache: 'no-store' }); @@ -response = await fetch(WEB_HEALTH_URL, { signal: controller.signal }); +response = await fetch(WEB_HEALTH_URL, { signal: controller.signal, cache: 'no-store' });Also applies to: 185-186
221-224: Optional unmount safety.Consider guarding setStatus with an isMounted ref to avoid updates after unmount if a fetch resolves late.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (2)
src/features/browser/hooks/useBrowser.ts(8 hunks)src/features/session/ui/MessageItem.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/features/session/ui/MessageItem.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/features/browser/hooks/useBrowser.ts (3)
src/features/browser/hooks/index.ts (1)
useBrowser(1-1)src/features/browser/index.ts (1)
useBrowser(2-2)backend/lib/message-sanitizer.cjs (1)
errorMessage(118-118)
🔇 Additional comments (3)
src/features/browser/hooks/useBrowser.ts (3)
51-57: Dev-only, scrubbed path logging: LGTM.This addresses prior PII/path leakage concerns and keeps logs dev-scoped.
Also applies to: 61-63
118-123: Safer error logging: LGTM.Minimal summaries avoid circular JSON and sensitive detail leakage.
Also applies to: 147-149
205-211: The review comment is based on an incorrect assumption and should be disregarded.The
devBrowserStatus.errorfield exists in the BrowserStatus type, and while the component checks fordevBrowserStatus.errorto prevent auto-start, it is not rendered in the UI. The component maintains a separate localerrorstate (line 27) that handles all user-facing error displays. The UI error overlay (lines 589-594) only renders this local error state, which is populated by the component's own handlers (navigation failures, iframe errors, injection failures), not from the hook's periodic poll errors.Likely an incorrect or invalid review comment.
…rkspaceItem.tsx 123 lines) from repository/ui that were never rendered anywhere, removed 14 dead barrel re-exports across 4 barrel files (repository/ui, sidebar/ui, sidebar feature, session/ui/blocks), and cleaned up knip.json — net reduction of 242 lines across 7 files with all 825 tests passing and clean tsc.
* gnhf #1: Extracted two helper methods in claude-adapter.ts (closeActiveParts, accumulateStreamDelta) to eliminate 5 repeated close-text/close-thinking call sites and 2 duplicate 25-line streaming delta handlers; also replaced a duplicate 15-line formatTime function in WorkspaceItem.tsx with the existing shared formatTimeAgo utility — net reduction of 49 lines with all 375 agent-server tests passing. * gnhf #2: Extracted duplicate workspace-grouping logic (85 lines across query-engine.ts and workspaces.ts route) into shared lib/workspace-grouping.ts, and consolidated duplicate parameter-reading helpers (readString/readNumber across query-engine.ts and commands.ts) into shared lib/query-params.ts — net reduction of 41 lines with all 825 tests passing. * gnhf #3: Removed unused parameter from cancellation functions across both agent handlers, extracted helper replacing 5 identical repo-lookup patterns in repos.ts, unified duplicate git progress push functions, and removed dead export — net reduction of 23 lines across 7 files with all 825 tests passing. * gnhf #4: Removed 5 dead exports/functions and un-exported 2 internal-only functions across 6 files — net reduction of 112 lines with all 825 tests passing. * gnhf #5: Removed 2 dead exported functions (getRepoInitials, getRepoColor), un-exported 3 internal-only symbols (RECENT_PROJECT_LIMIT, resolveGitProjectRoot, setLastOpenInAppId), and replaced duplicate timeAgo in AccessSection.tsx with shared formatTimeAgo — net reduction of 41 lines across 5 files with all 825 tests passing. * gnhf #6: Un-exported 14 dead Zod schema validators (6 from shared/events.ts, 8 from shared/agent-events.ts), un-exported 6 dead type aliases from shared/events.ts, and consolidated duplicate parseGitHubRepo function from gh.service.ts and deus-import.ts into shared/lib/github.ts — reducing public API surface by 20 exports and eliminating 1 duplicate function across 5 files with all 825 tests passing. * gnhf #7: Deleted 2 dead component files (OpenInDropdown 214 lines, EmptyState 39 lines), removed 3 dead API type definitions (ApiResponse, PaginatedResponse, WorkspaceQueryParams) from shared/types/api.ts, removed 4 dead session type aliases (SessionMessageEvent, SessionErrorEvent, SessionEnterPlanModeEvent, SessionStatusEvent) from shared/types/session.ts, and cleaned up 3 barrel re-export files — net reduction of 330 lines across 7 files with all 825 tests passing. * gnhf #8: Deleted 2 dead platform files (updates.ts 31 lines, listenerGroup.ts 38 lines), removed dead function from dialog.ts, removed the entire dead StatusChanged notification pipeline across 5 files (method, schema, type, constant, test builder, union), un-exported from electron barrel, and cleaned up 3 barrel re-export files — net reduction of 121 lines across 11 files with all 825 tests passing. * gnhf #9: Removed 3 dead query hooks (useStats, useUncommittedFiles, useLastTurnFiles) with their stub service methods and query keys, removed 4 dead type definitions (ChangedFilesResult, BranchInfo, PaginationParams, DevServer) and their barrel re-exports, un-exported internal-only connectToRelay function, and cleaned up 2 dead barrel re-exports (clearToken, ConnectionIllustration) — net reduction of 121 lines across 12 files with all 825 tests passing and clean tsc. * gnhf #10: Removed 6 dead visual effect builder functions (98 lines) from visual-effects.ts, un-exported 5 internal-only symbols (resolveClaudeDir, getAgentConfig, StatusPriority, StatusConfig, WorkflowStatusConfig), and removed dead barrel re-exports (createAgentEventHandler, AgentEventHandler) from agent/index.ts — net reduction of 98 lines across 5 files with all 825 tests passing and clean tsc. * gnhf #11: Consolidated inline path validation in files.ts to use shared resolveWorkspaceRelativePath from git.service.ts, and replaced 10 hand-rolled readString+throw param validation patterns in commands.ts with the existing requireParam utility — net reduction of 17 lines across 2 files with all 825 tests passing and clean tsc. * gnhf #12: Removed 211 lines of dead CSS from global.css: the entire glitch-swap effect system (6 classes, 6 @Keyframes, 2 media queries) and the empty tool-use-enter class — none referenced by any component, with all 825 tests passing. * gnhf #13: Deleted 2 dead component files (RepoGroup.tsx 102 lines, WorkspaceItem.tsx 123 lines) from repository/ui that were never rendered anywhere, removed 14 dead barrel re-exports across 4 barrel files (repository/ui, sidebar/ui, sidebar feature, session/ui/blocks), and cleaned up knip.json — net reduction of 242 lines across 7 files with all 825 tests passing and clean tsc. * gnhf #14: Deleted the dead agent-server/messages/index.ts barrel (30 lines), removed 4 dead type exports from shared/messages/types.ts (ToolLocation, ToolOutputContent, PartType, PartTypeSchema), removed dead ToolResultMap type from chat-types.ts, and cleaned up 15 dead barrel re-exports across 4 barrel files (session/hooks, session/ui, workspace/ui, shared/hooks) — net reduction of 51 lines across 8 files with all 825 tests passing and clean tsc. * gnhf #15: Removed 10 dead type aliases from shared/agent-events.ts, un-exported 2 dead Zod schemas from shared/enums.ts, removed the dead useWindowFocus hook and its useSyncExternalStore infrastructure (50 lines) from useWindowFocus.ts, and removed dead BaseToolRendererProps barrel re-export — net reduction of 62 lines across 4 files with all 825 tests passing and clean tsc. * refactor: extract backend helpers and fix typecheck * Address CodeRabbit review feedback * Fix Claude adapter message handoff
…rkspaceItem.tsx 123 lines) from repository/ui that were never rendered anywhere, removed 14 dead barrel re-exports across 4 barrel files (repository/ui, sidebar/ui, sidebar feature, session/ui/blocks), and cleaned up knip.json — net reduction of 242 lines across 7 files with all 825 tests passing and clean tsc.
1 participant
Overview
This PR refactors the frontend from a flat structure to a domain-driven, feature-first architecture (FSD-Lite) for better code organization and maintainability.
Changes
Structure
Testing
Migration Notes
🤖 Generated with Claude Code
Summary by CodeRabbit
Refactor
New Features / UX
Bug Fixes
Documentation