Conversation
|
Warning Rate limit exceeded@Kitenite has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 11 minutes and 24 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis PR refactors the terminal history and manager subsystems to use string scrollback instead of arrays, extracts a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Terminal Client
participant Manager as TerminalManager
participant History as HistoryWriter
participant Disk as Disk Storage
Note over Client,Disk: Old Flow (string[])
Client->>Manager: createOrAttach()
Manager->>Manager: Recover scrollback as string[]
Manager->>History: Per-chunk writes
History->>Disk: Append chunks to history.ndjson
Manager->>History: finalize(exitCode)
History->>Disk: Write metadata
Note over Client,Disk: New Flow (string)
Client->>Manager: createOrAttach()
Manager->>Manager: Recover scrollback as string
Manager->>History: init(recoveredScrollback)
History->>Disk: Write entire scrollback.bin
Manager->>History: write(data)
History->>Disk: Append to scrollback.bin
Manager->>History: close(exitCode)
History->>Disk: Write metadata to meta.json
sequenceDiagram
participant Router as Workspaces Router
participant Util as Git Utility
participant Git as Git Command
participant DB as Database
Note over Router,DB: Deletion Flow
Router->>Util: worktreeExists(mainRepoPath, path)
Util->>Git: git worktree list --porcelain
Git-->>Util: Worktree lines
Util-->>Router: boolean exists
alt Worktree exists
Router->>Git: Remove worktree
Router->>DB: Delete from database
else Worktree not found
Router->>DB: Delete from database
Router-->>Router: Return warning
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Areas requiring extra attention:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
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: 0
🧹 Nitpick comments (5)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupTabView/GroupTabPane.tsx (1)
61-61: Minor: Leading space in className.There's a leading space in the className string that could be cleaned up for consistency.
- className=" hover:text-white/80" + className="hover:text-white/80"apps/desktop/test-setup.ts (1)
16-27: Consider resettingmockStoragebetween tests to avoid state leakage
mockStorageis a module‑levelMapthat is never cleared, so values written by one test can affect subsequent tests usingwindow.electronStore. Consider exposing a reset helper (e.g.,mockStorage.clear()) and calling it in abeforeEachto keep tests independent and repeatable.
Based on learnings, tests should be independent and repeatable.apps/desktop/src/shared/constants/project-colors.ts (1)
1-10: Type annotation conflicts withas constassertion.The explicit type annotation
{ name: string; value: string }[]widens the type to a mutable array, which conflicts with theas constassertion on line 10. This makes theas consteffectively redundant.Consider one of these approaches:
Option 1: Remove explicit type (preferred for const narrowing):
-export const PROJECT_COLORS: { name: string; value: string }[] = [ +export const PROJECT_COLORS = [ { name: "Blue", value: "#3b82f6" }, ... ] as const;Option 2: Remove
as constif mutable array type is intended:-] as const; +];apps/desktop/src/main/lib/terminal-manager.ts (1)
139-142: Consider performance implications for long-running sessions.String concatenation with
+=creates a new string on each write, which may become inefficient for very long terminal sessions with thousands of output events. For typical usage this is likely fine, but for edge cases consider whether buffering or a different accumulation strategy might be beneficial.apps/desktop/src/main/lib/terminal-history.ts (1)
78-82: Consider guarding against writes after close.The
writemethod silently no-ops ifstreamis null, which happens afterclose(). This is acceptable for the current usage pattern, but you may want to log a warning or throw ifwriteis called aftercloseto catch programming errors.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
apps/desktop/src/lib/trpc/routers/notifications.ts(1 hunks)apps/desktop/src/lib/trpc/routers/projects/projects.ts(1 hunks)apps/desktop/src/lib/trpc/routers/projects/utils/colors/colors.ts(1 hunks)apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts(1 hunks)apps/desktop/src/lib/trpc/routers/workspaces/workspaces.test.ts(1 hunks)apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts(3 hunks)apps/desktop/src/main/lib/agent-setup.ts(1 hunks)apps/desktop/src/main/lib/terminal-history.test.ts(6 hunks)apps/desktop/src/main/lib/terminal-history.ts(5 hunks)apps/desktop/src/main/lib/terminal-manager.test.ts(4 hunks)apps/desktop/src/main/lib/terminal-manager.ts(11 hunks)apps/desktop/src/main/windows/main.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupTabView/GroupTabPane.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx(1 hunks)apps/desktop/src/shared/constants/project-colors.ts(1 hunks)apps/desktop/src/shared/ipc-channels/external.ts(1 hunks)apps/desktop/test-setup.ts(1 hunks)apps/website/src/app/components/Header/Header.tsx(1 hunks)packages/ui/src/components/alert-dialog.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Apply Biome formatting, linting, and import organization at the root level for all code files
Files:
apps/desktop/src/shared/ipc-channels/external.tsapps/desktop/src/lib/trpc/routers/projects/utils/colors/colors.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/git.tsapps/desktop/src/shared/constants/project-colors.tsapps/desktop/src/lib/trpc/routers/workspaces/workspaces.tsapps/desktop/src/main/windows/main.tsapps/desktop/src/lib/trpc/routers/notifications.tsapps/website/src/app/components/Header/Header.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupTabView/GroupTabPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/lib/trpc/routers/workspaces/workspaces.test.tsapps/desktop/src/main/lib/terminal-manager.test.tsapps/desktop/test-setup.tsapps/desktop/src/main/lib/terminal-history.test.tsapps/desktop/src/main/lib/agent-setup.tsapps/desktop/src/lib/trpc/routers/projects/projects.tsapps/desktop/src/main/lib/terminal-history.tspackages/ui/src/components/alert-dialog.tsxapps/desktop/src/main/lib/terminal-manager.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid usinganytype; use type-safe approaches instead, unless necessary
Ensure TypeScript type checking passes across all packages using bun run typecheck
Files:
apps/desktop/src/shared/ipc-channels/external.tsapps/desktop/src/lib/trpc/routers/projects/utils/colors/colors.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/git.tsapps/desktop/src/shared/constants/project-colors.tsapps/desktop/src/lib/trpc/routers/workspaces/workspaces.tsapps/desktop/src/main/windows/main.tsapps/desktop/src/lib/trpc/routers/notifications.tsapps/website/src/app/components/Header/Header.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupTabView/GroupTabPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/lib/trpc/routers/workspaces/workspaces.test.tsapps/desktop/src/main/lib/terminal-manager.test.tsapps/desktop/test-setup.tsapps/desktop/src/main/lib/terminal-history.test.tsapps/desktop/src/main/lib/agent-setup.tsapps/desktop/src/lib/trpc/routers/projects/projects.tsapps/desktop/src/main/lib/terminal-history.tspackages/ui/src/components/alert-dialog.tsxapps/desktop/src/main/lib/terminal-manager.ts
apps/**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
apps/**/*.{tsx,ts}: Structure React applications with one folder per component following the pattern: ComponentName/ComponentName.tsx with index.ts barrel export
Co-locate component dependencies (hooks, utils, constants, tests, stories) next to the file using them
Files:
apps/desktop/src/shared/ipc-channels/external.tsapps/desktop/src/lib/trpc/routers/projects/utils/colors/colors.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/git.tsapps/desktop/src/shared/constants/project-colors.tsapps/desktop/src/lib/trpc/routers/workspaces/workspaces.tsapps/desktop/src/main/windows/main.tsapps/desktop/src/lib/trpc/routers/notifications.tsapps/website/src/app/components/Header/Header.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupTabView/GroupTabPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/lib/trpc/routers/workspaces/workspaces.test.tsapps/desktop/src/main/lib/terminal-manager.test.tsapps/desktop/test-setup.tsapps/desktop/src/main/lib/terminal-history.test.tsapps/desktop/src/main/lib/agent-setup.tsapps/desktop/src/lib/trpc/routers/projects/projects.tsapps/desktop/src/main/lib/terminal-history.tsapps/desktop/src/main/lib/terminal-manager.ts
apps/desktop/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
For Electron interprocess communication, ALWAYS use tRPC as defined in
src/lib/trpc
Files:
apps/desktop/src/shared/ipc-channels/external.tsapps/desktop/src/lib/trpc/routers/projects/utils/colors/colors.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/git.tsapps/desktop/src/shared/constants/project-colors.tsapps/desktop/src/lib/trpc/routers/workspaces/workspaces.tsapps/desktop/src/main/windows/main.tsapps/desktop/src/lib/trpc/routers/notifications.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupTabView/GroupTabPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/lib/trpc/routers/workspaces/workspaces.test.tsapps/desktop/src/main/lib/terminal-manager.test.tsapps/desktop/test-setup.tsapps/desktop/src/main/lib/terminal-history.test.tsapps/desktop/src/main/lib/agent-setup.tsapps/desktop/src/lib/trpc/routers/projects/projects.tsapps/desktop/src/main/lib/terminal-history.tsapps/desktop/src/main/lib/terminal-manager.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: Please use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary
Files:
apps/desktop/src/shared/ipc-channels/external.tsapps/desktop/src/lib/trpc/routers/projects/utils/colors/colors.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/git.tsapps/desktop/src/shared/constants/project-colors.tsapps/desktop/src/lib/trpc/routers/workspaces/workspaces.tsapps/desktop/src/main/windows/main.tsapps/desktop/src/lib/trpc/routers/notifications.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupTabView/GroupTabPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/lib/trpc/routers/workspaces/workspaces.test.tsapps/desktop/src/main/lib/terminal-manager.test.tsapps/desktop/test-setup.tsapps/desktop/src/main/lib/terminal-history.test.tsapps/desktop/src/main/lib/agent-setup.tsapps/desktop/src/lib/trpc/routers/projects/projects.tsapps/desktop/src/main/lib/terminal-history.tsapps/desktop/src/main/lib/terminal-manager.ts
apps/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use one component per file; do not create multi-component files
Files:
apps/website/src/app/components/Header/Header.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupTabView/GroupTabPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
apps/desktop/src/renderer/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Define keyboard shortcuts using the centralized system in apps/desktop/src/renderer/lib/shortcuts.ts with Arc Browser-inspired shortcut definitions
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupTabView/GroupTabPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
apps/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Co-locate tests with their corresponding components (e.g., Component.test.tsx next to Component.tsx)
Files:
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.test.tsapps/desktop/src/main/lib/terminal-manager.test.tsapps/desktop/src/main/lib/terminal-history.test.ts
apps/desktop/**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.test.{ts,tsx,js,jsx}: Tests should have one assert per test
Tests should be readable
Tests should be fast
Tests should be independent
Tests should be repeatable
Files:
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.test.tsapps/desktop/src/main/lib/terminal-manager.test.tsapps/desktop/src/main/lib/terminal-history.test.ts
packages/ui/**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use TailwindCSS v4 and shadcn/ui for UI component styling and design
Files:
packages/ui/src/components/alert-dialog.tsx
🧠 Learnings (13)
📚 Learning: 2025-11-24T21:32:46.559Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T21:32:46.559Z
Learning: Applies to apps/desktop/src/shared/ipc-channels.ts : Define all Electron IPC channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers
Applied to files:
apps/desktop/src/shared/ipc-channels/external.tsapps/desktop/src/main/windows/main.ts
📚 Learning: 2025-11-24T21:32:46.559Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T21:32:46.559Z
Learning: Applies to apps/desktop/src/main/lib/*-ipcs.ts : Implement Electron IPC handlers in dedicated files under apps/desktop/src/main/lib/ (e.g., workspace-ipcs.ts, terminal-ipcs.ts)
Applied to files:
apps/desktop/src/main/windows/main.tsapps/desktop/src/lib/trpc/routers/notifications.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/lib/trpc/routers/workspaces/workspaces.test.tsapps/desktop/test-setup.tsapps/desktop/src/main/lib/agent-setup.tsapps/desktop/src/main/lib/terminal-history.ts
📚 Learning: 2025-11-24T21:32:46.559Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T21:32:46.559Z
Learning: Applies to @(apps/desktop/src/renderer|apps/desktop/src/shared)/**/*.{ts,tsx} : Never import Node.js modules in Electron renderer process or shared code; use IPC for communication between main and renderer processes
Applied to files:
apps/desktop/src/main/windows/main.ts
📚 Learning: 2025-11-24T21:32:46.559Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T21:32:46.559Z
Learning: Applies to apps/desktop/src/lib/electron-router-dom.ts : Do not import Node.js modules in apps/desktop/src/lib/electron-router-dom.ts as it is shared between main and renderer processes
Applied to files:
apps/desktop/src/lib/trpc/routers/notifications.tsapps/desktop/src/lib/trpc/routers/workspaces/workspaces.test.tsapps/desktop/src/main/lib/agent-setup.tsapps/desktop/src/lib/trpc/routers/projects/projects.ts
📚 Learning: 2025-11-24T21:33:13.244Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.244Z
Learning: Applies to apps/desktop/**/*.{ts,tsx,js,jsx} : For Electron interprocess communication, ALWAYS use tRPC as defined in `src/lib/trpc`
Applied to files:
apps/desktop/src/lib/trpc/routers/notifications.ts
📚 Learning: 2025-11-24T21:32:46.559Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T21:32:46.559Z
Learning: Applies to apps/**/*.{tsx,ts} : Co-locate component dependencies (hooks, utils, constants, tests, stories) next to the file using them
Applied to files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsxapps/desktop/src/lib/trpc/routers/workspaces/workspaces.test.ts
📚 Learning: 2025-11-24T21:33:13.244Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.244Z
Learning: Applies to apps/desktop/**/*.test.{ts,tsx,js,jsx} : Tests should be independent
Applied to files:
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.test.tsapps/desktop/test-setup.ts
📚 Learning: 2025-11-24T21:33:13.244Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.244Z
Learning: Applies to apps/desktop/**/*.test.{ts,tsx,js,jsx} : Tests should be repeatable
Applied to files:
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.test.tsapps/desktop/test-setup.ts
📚 Learning: 2025-11-24T21:32:46.559Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T21:32:46.559Z
Learning: Applies to apps/**/*.test.{ts,tsx} : Co-locate tests with their corresponding components (e.g., Component.test.tsx next to Component.tsx)
Applied to files:
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.test.ts
📚 Learning: 2025-11-24T21:33:13.244Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.244Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Please use alias as defined in `tsconfig.json` when possible
Applied to files:
apps/desktop/test-setup.tsapps/desktop/src/main/lib/agent-setup.ts
📚 Learning: 2025-11-24T21:33:13.244Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.244Z
Learning: Applies to apps/desktop/**/*.test.{ts,tsx,js,jsx} : Tests should be fast
Applied to files:
apps/desktop/test-setup.ts
📚 Learning: 2025-11-24T21:33:13.244Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.244Z
Learning: Applies to apps/desktop/**/*.test.{ts,tsx,js,jsx} : Tests should be readable
Applied to files:
apps/desktop/test-setup.ts
📚 Learning: 2025-11-24T21:32:46.559Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T21:32:46.559Z
Learning: Applies to apps/desktop/src/main/index.ts : Load environment variables from monorepo root .env file in apps/desktop/src/main/index.ts before any imports with override: true
Applied to files:
apps/desktop/src/main/lib/agent-setup.ts
🧬 Code graph analysis (3)
apps/desktop/src/lib/trpc/routers/projects/utils/colors/colors.ts (1)
apps/desktop/src/shared/constants/project-colors.ts (1)
PROJECT_COLOR_VALUES(12-12)
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (1)
apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts (2)
worktreeExists(99-116)removeWorktree(68-81)
apps/desktop/src/main/lib/terminal-manager.ts (1)
apps/desktop/src/main/lib/terminal-history.ts (2)
HistoryWriter(40-99)HistoryReader(101-135)
🔇 Additional comments (37)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupTabView/GroupTabPane.tsx (1)
1-2: LGTM!The import reorganization follows proper ordering conventions and aligns with the Biome import organization guidelines. External library imports are correctly placed at the top.
apps/desktop/src/shared/ipc-channels/external.ts (1)
7-7: Change is correct and safe: no code extendsExternalChannelsinterface.Verification confirms:
- File
apps/desktop/src/shared/ipc-channels/external.tscontains only the type alias on line 7- Comment explicitly states external operations now use tRPC and file exists for backwards compatibility
- No code in the codebase extends, augments, or otherwise depends on extending
ExternalChannels- Converting from
interfacetotypeprevents accidental extension of a deprecated interface, which aligns with the deprecation intentThe change is sound and consistent with the coding guideline to use tRPC for Electron interprocess communication.
apps/desktop/test-setup.ts (1)
51-57: BrowserWindow mock refactor looks behaviorally identicalSwitching to
mock(() => ({ ... }))keeps the same returned shape and preserves how tests will interact withBrowserWindow, includingwebContents.send,loadURL, andon. No issues from this change alone.packages/ui/src/components/alert-dialog.tsx (1)
2-2: Type-only React import is correctly implemented.The change from
import * as Reacttoimport type * as Reactis verified as correct. All 11 usages of React in the file are type annotations (React.ComponentProps), and the file uses the modern JSX transform ("jsx": "react-jsx"in tsconfig), which doesn't require React in scope for JSX to work. This optimization safely reduces the bundle size without affecting functionality.apps/desktop/src/main/lib/agent-setup.ts (1)
1-1: ConsolidatedexecSyncimport is correctSingle top-level import removes duplication without changing behavior.
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx (1)
7-8: Import reordering is fineMoving the icon import under the React hook keeps imports tidy without affecting behavior.
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.test.ts (1)
1-1: Singlebun:testimport is appropriateConsolidating the
bun:testimport at the top avoids duplication and keeps the test setup clear.apps/website/src/app/components/Header/Header.tsx (1)
7-9: Header import order adjustment is harmlessReordering the CTA-related imports is stylistic only and doesn’t affect the Header behavior.
apps/desktop/src/main/lib/terminal-manager.test.ts (1)
90-95: Tests correctly updated for string-based scrollbackAligning expectations to:
- Treat
scrollbackas a string (""initially,toContain(...)for history),- Loosen spawn’s argv assertion to
expect.any(Array),keeps the tests in sync with the new terminal history representation while still validating recovery semantics across sessions.
Also applies to: 376-378, 579-613
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx (1)
17-51: SimplifiedtabToRenderlogic looks correctThe memoized
tabToRendercomputation (including parent-group resolution and null fallbacks) and the directTabType-based rendering cleanly select a single tab view while still wiring the drop zone and EmptyTabView correctly when no active tab exists.apps/desktop/src/lib/trpc/routers/projects/projects.ts (1)
7-7:PROJECT_COLOR_VALUESimport placement is appropriateBringing the constant import up with other module imports is consistent organization and doesn’t alter router behavior.
apps/desktop/src/lib/trpc/routers/notifications.ts (1)
2-5: Destructured import reorder is safeReordering
AgentCompleteEventandnotificationsEmitterin the import has no effect on the TRPC notifications subscription.apps/desktop/src/lib/trpc/routers/projects/utils/colors/colors.ts (1)
3-6: LGTM!The formatting change is cosmetic only and the random color selection logic remains correct.
apps/desktop/src/main/windows/main.ts (1)
8-13: LGTM!Import reordering with proper
typeannotation forAgentCompleteEventfollows TypeScript best practices.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx (1)
105-111: LGTM!The scrollback type change from
string[]tostringcorrectly fixes the terminal history recovery. Writing the full scrollback string restores the complete terminal state rather than just the first line.apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts (1)
99-116: LGTM with a minor consideration.The implementation correctly parses the porcelain format and checks for worktree existence. The error handling is appropriate.
One potential edge case: the exact string match on line 111 may fail if paths differ in trailing slashes, case sensitivity (on case-insensitive filesystems), or symlink resolution. If you encounter issues, consider normalizing paths before comparison:
import { resolve } from "node:path"; // Inside worktreeExists: const normalizedWorktreePath = resolve(worktreePath); const worktreePrefix = `worktree ${normalizedWorktreePath}`; return lines.some((line) => { const match = line.trim().match(/^worktree (.+)$/); return match && resolve(match[1]) === normalizedWorktreePath; });apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (3)
12-12: LGTM!Clean import of the new
worktreeExistsutility, which properly encapsulates the git worktree existence check logic.
260-273: LGTM!The
canDeletequery now uses the extractedworktreeExistsutility, providing cleaner separation of concerns. The warning message for missing worktrees is helpful for debugging cases where worktrees were manually removed.
316-327: LGTM!Good defensive programming - checking existence before attempting removal prevents errors when worktrees were manually deleted. The warning log provides useful debugging context without failing the operation.
apps/desktop/src/main/lib/terminal-history.test.ts (9)
31-50: LGTM!Good end-to-end test validating the write-read cycle. The test properly initializes the writer, writes data, closes it, then verifies via reader.
52-74: LGTM!Comprehensive metadata verification covering all fields. The test correctly validates that metadata is properly persisted and retrieved.
76-108: LGTM!Excellent test for the append/recovery flow. This validates the critical path where a session is recovered and new data is appended, ensuring history continuity across sessions.
110-131: LGTM!Important test for ANSI escape code preservation. Terminal output often contains escape sequences for colors, cursor movement, and screen clearing - this ensures they survive the write/read cycle.
133-157: LGTM!Good stress test simulating realistic terminal output patterns with many small chunks. This validates the streaming write behavior handles high-frequency writes correctly.
159-180: LGTM!Important edge case test for binary-like data including null bytes, unicode, and control characters that may appear in terminal output.
205-211: LGTM!Proper test for the non-existent history case, verifying graceful handling with empty scrollback and undefined metadata.
213-234: LGTM!Good test for large scrollback handling. Verifying 200KB of data is read without truncation ensures the implementation doesn't have arbitrary limits.
256-262: Acceptable use of@ts-expect-errorfor ENOENT check.The pattern of catching the error and checking
error.codeforENOENTis appropriate for verifying directory deletion. The@ts-expect-errordirective is necessary because the error type isunknown.apps/desktop/src/main/lib/terminal-manager.ts (4)
16-16: LGTM!Changing
scrollbackfromstring[]tostringsimplifies the API surface and aligns with how terminal data is naturally produced and consumed.
70-102: LGTM!Good recovery strategy - prioritizing in-memory scrollback from dead sessions enables fast reconnection without disk I/O, while falling back to disk for cold starts.
145-156: LGTM!The exit handler properly closes history before emitting the exit event, and the 5-second timeout for session cleanup allows brief reconnection windows.
304-326: LGTM!The
closeHistorymethod properly handles both cleanup scenarios:
- When
deleteHistoryOnExitis true: closes the stream first, then removes the history directory- Otherwise: closes with exit code to persist metadata
Setting
historyWritertoundefinedafter close prevents double-close issues.apps/desktop/src/main/lib/terminal-history.ts (5)
1-2: LGTM!Clean imports for the streaming file write pattern. Using
createWriteStreamfor append operations andpromisesAPI for other fs operations is idiomatic.
32-38: Good encapsulation - path helpers are now private.Making
getHistoryFilePathandgetMetadataPathprivate (non-exported) improves encapsulation. External code should useHistoryReader/HistoryWriterrather than directly accessing file paths.
63-76: LGTM!Good approach for the init method:
- Create directory recursively
- Write initial scrollback (or empty file) to establish the file
- Open stream in append mode for subsequent writes
This ensures recovered scrollback is preserved and new data is appended.
84-98: LGTM!The
closemethod properly:
- Waits for the stream to fully flush and close
- Sets stream to null to prevent further writes
- Updates metadata with
endedAtandexitCode- Persists metadata to disk
The promise-based stream ending pattern is correct.
107-124: LGTM!The
readmethod gracefully handles:
- Missing scrollback file (returns empty string)
- Missing or corrupt metadata (returns undefined metadata)
This fail-safe behavior is appropriate for optional history recovery.
1 participant
Summary by CodeRabbit
Refactor
Style
Tests
✏️ Tip: You can customize this high-level summary in your review settings.