Skip to content

conditional render#152

Merged
Kitenite merged 5 commits intomainfrom
conditional-render
Nov 26, 2025
Merged

conditional render#152
Kitenite merged 5 commits intomainfrom
conditional-render

Conversation

@Kitenite
Copy link
Copy Markdown
Collaborator

@Kitenite Kitenite commented Nov 26, 2025

Summary by CodeRabbit

  • Refactor

    • Simplified tab area to render a single active view, improved empty-state handling, and streamlined group/parent tab presentation with a consistent drop overlay.
  • Bug Fix

    • Safer workspace deletion: existence is now verified before attempting removal; missing items are skipped with a warning and cleanup proceeds only when appropriate.
  • Style / Chore

    • Minor formatting and typing cleanups for color data and shell command construction.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Nov 26, 2025

Warning

Rate limit exceeded

@Kitenite has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 52 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

📥 Commits

Reviewing files that changed from the base of the PR and between dadafa4 and 65eea9a.

📒 Files selected for processing (1)
  • apps/desktop/src/lib/trpc/routers/external/index.ts (1 hunks)

Walkthrough

UI: TabsContent was simplified to render a single resolved tab (resolves to parent when needed) and now directly branches to Setup/Single/Group views with a single fixed drop-overlay message. Backend: workspace deletion now uses a worktree existence check before attempting removal and skips removal if the worktree is missing.

Changes

Cohort / File(s) Summary
TabsContent (single-tab render)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx
Replaced multi-tab mapping with a tabToRender useMemo (resolves parent if active tab has parentId); removed renderTabContent; render now directly switches on tabToRender.type to render SetupTabView / SingleTabView / GroupTabView; empty-state container class adjusted; fixed DropOverlay message.
Workspaces router (worktree-safe delete)
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts
Replaced direct porcelain parsing with worktreeExists(...) check in canDelete and delete flows; if worktree absent, skip git removal with a logged warning and proceed to DB cleanup; only attempt removeWorktree when existence confirmed; improved error handling around existence/removal.
Git utils (new helper)
apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts
Added exported worktreeExists(mainRepoPath: string, worktreePath: string): Promise<boolean> that runs git worktree list --porcelain and parses for worktree <path> to determine presence; logs and rethrows on errors.
Project colors typing / small formatting tweaks
apps/desktop/src/shared/constants/project-colors.ts, apps/desktop/src/lib/trpc/routers/projects/utils/colors/colors.ts
PROJECT_COLORS now has an explicit type annotation { name: string; value: string }[]; minor formatting changes in assignRandomColor() return expression only.
Terminal manager formatting
apps/desktop/src/main/lib/terminal-manager.ts
Non-functional formatting change: multi-line construction of shell args for login shells.
Manifest
package.json
Included in manifest summary (no code-level change described in diff).

Sequence Diagram(s)

sequenceDiagram
    participant UI as TabsContent
    participant Memo as tabToRender useMemo
    participant View as Renderer

    UI->>Memo: compute activeTab (use parentId if present)
    Memo-->>View: tabToRender (parent or active)
    alt tabToRender == null
        View->>View: Render EmptyTabView (flex-1 h-full)
    else
        alt tab.type == "setup"
            View->>View: Render SetupTabView
        else tab.type == "single"
            View->>View: Render SingleTabView (isDropZone)
            View->>View: Show DropOverlay "Drop to create split view"
        else
            View->>View: Render GroupTabView
            View->>View: Show DropOverlay "Drop to create split view"
        end
    end
Loading
sequenceDiagram
    participant API as workspaces.delete
    participant GitList as git worktree list
    participant GitRemove as git worktree remove
    participant DB as database

    API->>GitList: worktreeExists(mainRepoPath, path)
    alt exists == true
        API->>GitRemove: removeWorktree(path)
        alt removal succeeds
            GitRemove-->>API: success
            API->>DB: cleanup workspace records
            DB-->>API: success
            API-->>Client: success
        else removal fails
            GitRemove-->>API: error -> API returns error
        end
    else exists == false
        API-->>API: log warning, skip removal
        API->>DB: cleanup workspace records
        DB-->>API: success
        API-->>Client: success (with warning)
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Areas to focus on:
    • Correctness and edge cases of tabToRender resolution (parentId handling, stale/undefined activeTab).
    • Integration with drag/drop hooks and whether the fixed DropOverlay message fits all drop targets.
    • Path normalization and platform differences in worktreeExists (relative vs absolute paths, symlinks).
    • Error propagation and DB cleanup guarantees when git removal fails or worktree is missing.
    • Tests or callers that assumed multiple tab content rendering — ensure no UI regressions.

Possibly related PRs

  • finish ui #106 — Changes to tab drop hooks and DropOverlay usage that directly interact with the TabsContent drop behavior.
  • tabs mangement interaction #105 — Another PR modifying TabsContent rendering and tab resolution; likely overlapping changes to single-tab logic.
  • fix worktree setup #146 — Related worktree/workspace lifecycle updates touching the workspaces router and git helpers.

Poem

🐰 I hop to the parent when a child slips away,
One tab to show, one message to say:
"Drop to create split view" — simple and neat,
I tidy the workspace with tiny feet. 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings, 1 inconclusive)
Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request description is completely empty and missing all required template sections including Description, Related Issues, Type of Change, Testing, and Additional Notes. Add a comprehensive description following the template. Include what changes were made, why, related issues, the type of change, and testing steps performed.
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title 'conditional render' is vague and generic, using non-descriptive terms that don't convey meaningful information about the specific changes across multiple files. Provide a more specific title that highlights the main change, such as 'Refactor tab rendering logic to use single-tab flow' or similar.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e43a802 and 678893d.

📒 Files selected for processing (1)
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{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/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Avoid using any type; use type-safe approaches instead, unless necessary
Ensure TypeScript type checking passes across all packages using bun run typecheck

Files:

  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx
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/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx
apps/**/*.tsx

📄 CodeRabbit inference engine (AGENTS.md)

Use one component per file; do not create multi-component files

Files:

  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.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.tsx
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/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx
apps/desktop/**/*.{ts,tsx}

📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)

apps/desktop/**/*.{ts,tsx}: Please use alias as defined in tsconfig.json when possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary

Files:

  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx
🧠 Learnings (1)
📚 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 : Use one component per file; do not create multi-component files

Applied to files:

  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx
🧬 Code graph analysis (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx (6)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/useTabContentDrop.ts (1)
  • useTabContentDrop (6-36)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/EmptyTabView.tsx (1)
  • EmptyTabView (1-18)
apps/desktop/src/shared/types.ts (1)
  • TabType (23-30)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/SingleTabView.tsx (1)
  • SingleTabView (16-49)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/DropOverlay.tsx (1)
  • DropOverlay (5-13)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupTabView/index.tsx (1)
  • GroupTabView (47-149)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Build
🔇 Additional comments (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx (2)

16-30: LGTM! Clean tab resolution logic.

The memoized logic correctly handles the hierarchy: when the active tab has a parent, it returns the parent group for rendering. Early null checks and proper dependencies make this robust.


34-40: LGTM! Appropriate empty state handling.

The empty state container correctly uses drop targeting and renders the EmptyTabView component when no tab is available.

Comment on lines +44 to +54
{tabToRender.type === TabType.Single ? (
<>
<SingleTabView tab={tabToRender} isDropZone={isDropZone} />
{isDropZone && <DropOverlay message="Drop to create split view" />}
</>
) : (
<>
<GroupTabView tab={tabToRender} />
{isDropZone && <DropOverlay message="Drop to add to split view" />}
</>
)}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: TabType.Single does not exist; invalid prop passed to SingleTabView.

Two critical issues:

  1. Line 44: TabType.Single is not a valid value in the TabType union. According to the type definition in apps/desktop/src/shared/types.ts, TabType only includes: "terminal", "editor", "browser", "preview", "group", "port", "diff". This condition will always be false or cause a TypeScript error.

  2. Line 46: isDropZone prop is passed to SingleTabView, but the component signature in SingleTabView.tsx only accepts tab: Tab. This will cause a type error.

Apply this diff to fix both issues:

-		{tabToRender.type === TabType.Single ? (
+		{tabToRender.type !== "group" ? (
 			<>
-				<SingleTabView tab={tabToRender} isDropZone={isDropZone} />
+				<SingleTabView tab={tabToRender} />
 				{isDropZone && <DropOverlay message="Drop to create split view" />}
 			</>
 		) : (
 			<>
 				<GroupTabView tab={tabToRender} />
 				{isDropZone && <DropOverlay message="Drop to add to split view" />}
 			</>
 		)}

Run this script to verify the TabType definition:

#!/bin/bash
# Description: Verify TabType definition to confirm "Single" is not a valid value

# Search for TabType definition
ast-grep --pattern 'export type TabType = $$$'

# Search for any usage of TabType.Single or "Single" as a tab type
rg -n "TabType\.Single|type:\s*['\"]Single['\"]" --type=ts --type=tsx
🤖 Prompt for AI Agents
In
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx
around lines 44-54, the code checks TabType.Single (which doesn't exist) and
passes an extra isDropZone prop to SingleTabView; change the conditional to test
for the group type (e.g. tabToRender.type === TabType.Group or tabToRender.type
!== 'group' depending on project convention) so SingleTabView branch runs for
non-group tabs, and remove the isDropZone prop from the SingleTabView call (keep
DropOverlay rendering as the separate conditional). Ensure imports/reference use
the actual TabType values defined in apps/desktop/src/shared/types.ts.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (1)

328-340: Consider extracting the worktree existence check into a helper function.

The worktree existence check logic (lines 328-340) is duplicated from the canDelete procedure (lines 260-273). Extracting this into a reusable helper function in the ./utils/git.ts module would reduce duplication and improve maintainability.

For example, add to apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts:

export async function worktreeExists(
	mainRepoPath: string,
	worktreePath: string,
): Promise<boolean> {
	const gitInstance = simpleGit(mainRepoPath);
	const worktrees = await gitInstance.raw([
		"worktree",
		"list",
		"--porcelain",
	]);

	const lines = worktrees.split("\n");
	const worktreePrefix = `worktree ${worktreePath}`;
	return lines.some((line) => line.trim() === worktreePrefix);
}

Then use it in both canDelete and delete:

-const gitInstance = simpleGit(project.mainRepoPath);
-const worktrees = await gitInstance.raw([
-	"worktree",
-	"list",
-	"--porcelain",
-]);
-
-const lines = worktrees.split("\n");
-const worktreePrefix = `worktree ${worktree.path}`;
-const worktreeExists = lines.some(
-	(line) => line.trim() === worktreePrefix,
-);
+const worktreeExists = await worktreeExists(project.mainRepoPath, worktree.path);
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 678893d and 73c2f06.

📒 Files selected for processing (1)
  • apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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/lib/trpc/routers/workspaces/workspaces.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Avoid using any type; use type-safe approaches instead, unless necessary
Ensure TypeScript type checking passes across all packages using bun run typecheck

Files:

  • apps/desktop/src/lib/trpc/routers/workspaces/workspaces.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/lib/trpc/routers/workspaces/workspaces.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/lib/trpc/routers/workspaces/workspaces.ts
apps/desktop/**/*.{ts,tsx}

📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)

apps/desktop/**/*.{ts,tsx}: Please use alias as defined in tsconfig.json when possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary

Files:

  • apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts
🧠 Learnings (1)
📚 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/lib/trpc/routers/workspaces/workspaces.ts
🧬 Code graph analysis (1)
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (1)
apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts (1)
  • removeWorktree (68-81)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Build
🔇 Additional comments (1)
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (1)

327-349: LGTM! Safe worktree removal with existence check.

The pre-removal existence check prevents errors when attempting to remove worktrees that have been manually deleted. The logic correctly parses the porcelain format and safely skips removal while still proceeding with DB cleanup.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx (1)

45-49: Critical: Invalid TabType enum usage and incorrect prop passed to SingleTabView.

Multiple critical issues that will cause TypeScript compilation errors:

  1. Lines 45, 46, 49: TabType.Setup, TabType.Single, and TabType.Group are used as if TabType is an enum, but according to the type definition in apps/desktop/src/renderer/types.ts, TabType is a union of string literals: "terminal" | "editor" | "browser" | "preview" | "group" | "port" | "diff". These values don't exist in the TabType union.

  2. Line 47: The isDropZone prop is passed to SingleTabView, but the component signature only accepts tab: Tab. This will cause a type error.

Based on the component purposes and the available TabType values, the logic should likely be:

  • Check for "group" type using string literal comparison
  • Render SingleTabView for all non-group types
  • Handle the DropOverlay separately (as already done on line 50)

Note: If "setup" is intended as a new tab type, it needs to be added to the TabType union definition first.

Apply this diff to fix the type checking and prop passing:

-		{tabToRender.type === TabType.Setup && <SetupTabView tab={tabToRender} />}
-		{tabToRender.type === TabType.Single && (
-			<SingleTabView tab={tabToRender} isDropZone={isDropZone} />
-		)}
-		{tabToRender.type === TabType.Group && <GroupTabView tab={tabToRender} />}
+		{tabToRender.type === "group" ? (
+			<GroupTabView tab={tabToRender} />
+		) : (
+			<SingleTabView tab={tabToRender} />
+		)}
 		{isDropZone && <DropOverlay message="Drop to create split view" />}

If "setup" is a valid tab type that needs special handling, first add it to the TabType union in apps/desktop/src/renderer/types.ts:

export type TabType =
	| "terminal"
	| "editor"
	| "browser"
	| "preview"
	| "group"
	| "port"
	| "diff"
	| "setup";

Then update the conditional to check for it:

-		{tabToRender.type === TabType.Setup && <SetupTabView tab={tabToRender} />}
-		{tabToRender.type === TabType.Single && (
-			<SingleTabView tab={tabToRender} isDropZone={isDropZone} />
-		)}
-		{tabToRender.type === TabType.Group && <GroupTabView tab={tabToRender} />}
+		{tabToRender.type === "setup" ? (
+			<SetupTabView tab={tabToRender} />
+		) : tabToRender.type === "group" ? (
+			<GroupTabView tab={tabToRender} />
+		) : (
+			<SingleTabView tab={tabToRender} />
+		)}
 		{isDropZone && <DropOverlay message="Drop to create split view" />}
🧹 Nitpick comments (3)
apps/desktop/src/shared/constants/project-colors.ts (1)

1-10: Type annotation conflicts with as const assertion.

The explicit type { name: string; value: string }[] widens the array type, making the as const assertion on line 10 ineffective. This inconsistency may lead to confusion.

Choose one approach:

  • Remove as const if you want the wider mutable type
  • Or remove the explicit type annotation to preserve literal types from as const
-export const PROJECT_COLORS: { name: string; value: string }[] = [
+export const PROJECT_COLORS = [
 	{ name: "Blue", value: "#3b82f6" },
 	// ... rest of colors
 ] as const;

Or if you prefer the wider type:

 export const PROJECT_COLORS: { name: string; value: string }[] = [
 	{ name: "Blue", value: "#3b82f6" },
 	// ... rest of colors
-] as const;
+];
apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts (1)

99-115: Consider normalizing paths for cross-platform compatibility.

The exact string comparison on line 111 may fail if:

  • Paths have trailing slashes
  • Path separators differ (Windows backslash vs forward slash)
  • Paths have different casing on case-insensitive filesystems

Consider normalizing both paths before comparison:

+import { normalize } from "node:path";
+
 export async function worktreeExists(
 	mainRepoPath: string,
 	worktreePath: string,
 ): Promise<boolean> {
 	try {
 		const git = simpleGit(mainRepoPath);
 		const worktrees = await git.raw(["worktree", "list", "--porcelain"]);
 
 		// Parse porcelain format to verify worktree exists
 		// Format: "worktree /path/to/worktree" followed by HEAD, branch, etc.
 		const lines = worktrees.split("\n");
-		const worktreePrefix = `worktree ${worktreePath}`;
-		return lines.some((line) => line.trim() === worktreePrefix);
+		const normalizedTarget = normalize(worktreePath);
+		return lines.some((line) => {
+			const trimmed = line.trim();
+			if (trimmed.startsWith("worktree ")) {
+				const listedPath = normalize(trimmed.slice(9));
+				return listedPath === normalizedTarget;
+			}
+			return false;
+		});
 	} catch (error) {
 		console.error(`Failed to check worktree existence: ${error}`);
 		throw error;
 	}
 }
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (1)

238-296: Minor: Consider consistent return shape for canDelete.

The return object has an optional warning field that's present in some branches but missing in others (e.g., lines 244-249 and 282-286). While TypeScript will infer a union type, explicit consistency can improve type safety for callers.

 if (!workspace) {
 	return {
 		canDelete: false,
 		reason: "Workspace not found",
 		workspace: null,
+		warning: null,
 	};
 }

And similarly for the error case at lines 282-286.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 73c2f06 and dadafa4.

📒 Files selected for processing (6)
  • 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.ts (3 hunks)
  • apps/desktop/src/main/lib/terminal-manager.ts (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)
✅ Files skipped from review due to trivial changes (2)
  • apps/desktop/src/lib/trpc/routers/projects/utils/colors/colors.ts
  • apps/desktop/src/main/lib/terminal-manager.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{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/lib/trpc/routers/workspaces/utils/git.ts
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx
  • apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts
  • apps/desktop/src/shared/constants/project-colors.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Avoid using any type; use type-safe approaches instead, unless necessary
Ensure TypeScript type checking passes across all packages using bun run typecheck

Files:

  • apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx
  • apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts
  • apps/desktop/src/shared/constants/project-colors.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/lib/trpc/routers/workspaces/utils/git.ts
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx
  • apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts
  • apps/desktop/src/shared/constants/project-colors.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/lib/trpc/routers/workspaces/utils/git.ts
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx
  • apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts
  • apps/desktop/src/shared/constants/project-colors.ts
apps/desktop/**/*.{ts,tsx}

📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)

apps/desktop/**/*.{ts,tsx}: Please use alias as defined in tsconfig.json when possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary

Files:

  • apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx
  • apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts
  • apps/desktop/src/shared/constants/project-colors.ts
apps/**/*.tsx

📄 CodeRabbit inference engine (AGENTS.md)

Use one component per file; do not create multi-component files

Files:

  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.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.tsx
🧠 Learnings (2)
📚 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/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx
  • apps/desktop/src/lib/trpc/routers/workspaces/workspaces.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/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx
🧬 Code graph analysis (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx (7)
apps/desktop/src/renderer/stores/tabs/hooks.ts (2)
  • useTabs (3-3)
  • useActiveTabIds (4-5)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/useTabContentDrop.ts (1)
  • useTabContentDrop (6-36)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/EmptyTabView.tsx (1)
  • EmptyTabView (1-18)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/SetupTabView.tsx (1)
  • SetupTabView (8-20)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/SingleTabView.tsx (1)
  • SingleTabView (16-49)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupTabView/index.tsx (1)
  • GroupTabView (47-149)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/DropOverlay.tsx (1)
  • DropOverlay (5-13)
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)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Build
🔇 Additional comments (3)
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (3)

12-12: LGTM!

Good addition of the worktreeExists import to centralize worktree existence checks.


260-273: LGTM - Improved existence check with helpful warning.

The new flow properly handles the case where a worktree may have been manually removed outside the application, returning canDelete: true with an informative warning.


316-327: Good defensive check before worktree removal.

The existence check before calling removeWorktree prevents errors when the worktree was already removed externally. The warning log provides useful debugging information.

@Kitenite Kitenite merged commit d772a66 into main Nov 26, 2025
2 of 5 checks passed
@Kitenite Kitenite deleted the conditional-render branch November 26, 2025 22:22
@coderabbitai coderabbitai Bot mentioned this pull request Nov 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant