Skip to content

emerald river 7#127

Merged
Kitenite merged 1 commit intomainfrom
emerald-river-7
Nov 22, 2025
Merged

emerald river 7#127
Kitenite merged 1 commit intomainfrom
emerald-river-7

Conversation

@Kitenite
Copy link
Copy Markdown
Collaborator

@Kitenite Kitenite commented Nov 22, 2025

Summary by CodeRabbit

  • New Features
    • Users can now rename workspaces by double-clicking on a workspace tab. An editable text input appears, allowing direct renaming with Enter to confirm or Escape to cancel.

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

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Nov 22, 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 15 minutes and 51 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 d37abe8 and a4a1bc7.

📒 Files selected for processing (2)
  • apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsx (4 hunks)
  • apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/useWorkspaceRename.ts (1 hunks)

Walkthrough

Most changes consist of import reordering, deduplication, and formatting across multiple files. A new workspace rename feature is introduced via a useWorkspaceRename hook and integrated into the WorkspaceItem component, enabling in-place renaming. Minor updates include @ts-ignore to @ts-expect-error and test mock refactoring.

Changes

Cohort / File(s) Summary
Import and deduplication updates
apps/desktop/src/lib/trpc/routers/notifications.ts, apps/desktop/src/main/lib/agent-setup.ts, apps/desktop/src/main/lib/terminal-manager.ts, apps/desktop/src/main/windows/main.ts
Reordered imports for consistency; deduplicated execSync import in agent-setup.ts; consolidated NOTIFICATIONS_PORT and type AgentCompleteEvent imports in main.ts
Workspace rename feature
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/useWorkspaceRename.ts, apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsx
New custom hook useWorkspaceRename managing rename state, submission, and cancellation; integrated into WorkspaceItem with double-click trigger and conditional text input rendering
Formatting and minor updates
apps/desktop/src/main/lib/terminal-history.test.ts, apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx
Updated @ts-ignore to @ts-expect-error; reordered test imports; reformatted HTML attribute placement
Test setup refactoring
apps/desktop/test-setup.ts
Changed BrowserWindow mock from function-returning object factory to arrow function syntax
Server startup refactoring
apps/desktop/src/main/windows/main.ts
Converted single-line notificationsApp.listen call to multiline format with explicit callback

Sequence Diagram

sequenceDiagram
    actor User
    participant WorkspaceItem
    participant useWorkspaceRename
    participant updateWorkspace

    User->>WorkspaceItem: Double-click workspace tab
    WorkspaceItem->>useWorkspaceRename: startRename()
    activate useWorkspaceRename
    useWorkspaceRename->>useWorkspaceRename: Set isRenaming=true, auto-select input
    useWorkspaceRename-->>WorkspaceItem: isRenaming=true, inputRef focused
    deactivate useWorkspaceRename
    
    User->>WorkspaceItem: Type new name
    WorkspaceItem->>useWorkspaceRename: setRenameValue(newName)
    
    User->>WorkspaceItem: Press Enter
    WorkspaceItem->>useWorkspaceRename: submitRename()
    activate useWorkspaceRename
    alt Name changed & non-empty
        useWorkspaceRename->>updateWorkspace: mutate({name: trimmedValue})
        updateWorkspace-->>useWorkspaceRename: Success
    else Name unchanged or empty
        useWorkspaceRename->>useWorkspaceRename: Reset to original
    end
    useWorkspaceRename->>useWorkspaceRename: Set isRenaming=false
    useWorkspaceRename-->>WorkspaceItem: Rename complete
    deactivate useWorkspaceRename
    WorkspaceItem-->>User: Display updated/original title
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Areas requiring attention:
    • New useWorkspaceRename hook implementation and state management logic (auto-select, trim, validation flow)
    • Integration of rename handler with double-click trigger and event propagation in WorkspaceItem.tsx
    • Verification that keyboard handlers (Enter/Escape) and blur behavior function correctly

Possibly related PRs

  • Set up hooks for notifications #121: Modifies the same notification-related and agent-setup files touched by this PR (notifications.ts, agent-setup.ts, terminal-manager.ts, main.ts), suggesting concurrent refactoring efforts.

Poem

A double-click now grants wishes true,
To rename workspaces, old names shed anew.
The rabbit hops through imports tidy,
While Enter confirms, Escape says "nope, nidi!"
🐰✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings, 1 inconclusive)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description is largely incomplete, lacking required template sections and providing minimal context about the changes. Fill in the description template with clear sections for Description, Type of Change, Testing, and other details required by the template.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.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 'emerald river 7' is vague and generic, providing no meaningful information about the changeset. Replace with a descriptive title summarizing the main change, such as 'Add workspace rename functionality and import cleanup' or similar.

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

@Kitenite Kitenite force-pushed the emerald-river-7 branch 2 times, most recently from 3e15d5d to d37abe8 Compare November 22, 2025 19:33
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@Kitenite Kitenite merged commit 84cf4be into main Nov 22, 2025
0 of 4 checks passed
@Kitenite Kitenite deleted the emerald-river-7 branch November 22, 2025 19:37
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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/desktop/src/main/lib/agent-setup.ts (1)

121-123: Fix getSupersetPath PATH separator for Windows

getSupersetPath currently hardcodes : as the PATH separator, which prevents BIN_DIR from being correctly added to PATH on Windows (where ; is required). This means the ~/.superset/bin wrappers (e.g., for Claude/Codex) won’t be on PATH in Windows terminals.

Consider making the separator platform-aware and avoiding a leading/trailing separator when PATH is empty:

-export function getSupersetPath(): string {
-	return `${BIN_DIR}:${process.env.PATH || ""}`;
-}
+export function getSupersetPath(): string {
+	const separator = os.platform() === "win32" ? ";" : ":";
+	const currentPath = process.env.PATH;
+
+	if (!currentPath || currentPath.length === 0) {
+		return BIN_DIR;
+	}
+
+	return `${BIN_DIR}${separator}${currentPath}`;
+}
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/useWorkspaceRename.ts (1)

4-63: Hook logic looks good; consider tightening the KeyboardEvent typing

The rename behavior (state, syncing with workspaceName, trim/no‑op logic, and Enter/Escape handling) is well-structured. One small improvement: handleKeyDown is typed as React.KeyboardEvent but React isn’t imported in this module. To avoid relying on a global React namespace (and keep things friendlier to newer React/TS setups), consider importing the type explicitly, for example:

import type { KeyboardEvent } from "react";

const handleKeyDown = (e: KeyboardEvent<HTMLInputElement>) => {
  // ...
};

This keeps the hook self-contained from a typing perspective.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ed9c965 and d37abe8.

📒 Files selected for processing (9)
  • apps/desktop/src/lib/trpc/routers/notifications.ts (1 hunks)
  • apps/desktop/src/main/lib/agent-setup.ts (1 hunks)
  • apps/desktop/src/main/lib/terminal-history.test.ts (2 hunks)
  • apps/desktop/src/main/lib/terminal-manager.ts (1 hunks)
  • apps/desktop/src/main/windows/main.ts (2 hunks)
  • apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsx (4 hunks)
  • apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/useWorkspaceRename.ts (1 hunks)
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx (1 hunks)
  • apps/desktop/test-setup.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
apps/desktop/src/main/windows/main.ts (2)
apps/desktop/src/main/lib/notifications/server.ts (2)
  • notificationsApp (56-56)
  • NOTIFICATIONS_PORT (57-57)
apps/desktop/src/shared/constants.ts (1)
  • NOTIFICATIONS_PORT (11-11)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsx (1)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/useWorkspaceRename.ts (1)
  • useWorkspaceRename (4-64)
🔇 Additional comments (9)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx (1)

155-161: Needs-attention badge formatting change looks good

Only JSX formatting changed for the attention indicator; behavior and semantics are preserved.

apps/desktop/src/main/lib/terminal-manager.ts (1)

1-6: Import reordering for getSupersetPath is fine

Moving getSupersetPath into the main import block is a no-op at runtime and keeps imports organized.

apps/desktop/src/main/lib/agent-setup.ts (1)

1-1: Top-level execSync import consolidation LGTM

Importing execSync once at the top cleans up the module and avoids duplicate/local imports without changing behavior.

apps/desktop/src/lib/trpc/routers/notifications.ts (1)

1-5: Import order tweak for notifications server is harmless

Reordering AgentCompleteEvent and notificationsEmitter within the same import has no effect on runtime; change is purely organizational.

apps/desktop/src/main/lib/terminal-history.test.ts (2)

4-11: Import grouping for history utilities looks good

Co-locating HistoryReader and HistoryWriter with related exports from ./terminal-history improves readability without changing test behavior.


382-384: Using @ts-expect-error instead of @ts-ignore is an improvement

Switching to // @ts-expect-error documents the intentional type error on error.code and will surface if the type definition is ever tightened, which is exactly what we want in this negative test.

apps/desktop/src/main/windows/main.ts (1)

9-13: Notifications server wiring and teardown look solid

Importing AgentCompleteEvent as a type and NOTIFICATIONS_PORT alongside notificationsApp keeps server concerns co-located, and capturing the return from notificationsApp.listen so it can be closed on window.on("close") avoids leaving a stray HTTP server running. The added log is also helpful for debugging without changing behavior.

Also applies to: 48-56

apps/desktop/test-setup.ts (1)

51-57: BrowserWindow mock refactor is behavior‑preserving

Changing the mock implementation to use an arrow function passed into mock(...) keeps the same returned object shape and call behavior while simplifying the definition. No issues from this change.

apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsx (1)

11-12: In‑place workspace rename flow is cleanly integrated

Using useWorkspaceRename(id, title) to own all rename state and handlers, wiring double‑click to startRename, and rendering a controlled <input> when isRenaming is true all look correct. The guards on onMouseDown plus stopping propagation from the input avoid conflicts with tab activation and drag/reorder behavior, while still preserving the existing attention indicator and delete dialog flows. No functional issues spotted here.

Also applies to: 41-41, 85-86, 100-123

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