refactor(desktop): migrate storage to tRPC, fix race condition#303
refactor(desktop): migrate storage to tRPC, fix race condition#303
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded@Kitenite has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 46 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 (3)
WalkthroughReplaces tab-centric identifiers with pane-centric ones across terminal, notifications, and state layers; removes electron-store IPC and renderer adapter; adds a centralized Lowdb-backed appState with a new tRPC uiState router and trpc-backed Zustand storage adapters; introduces shared tab/pane types. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Kitenite
force-pushed
the
literary-wren-cfeb4b
branch
from
1ae0af1 to
ad894b1
Compare
Kitenite
force-pushed
the
literary-wren-cfeb4b
branch
from
ad894b1 to
67130ba
Compare
Kitenite
force-pushed
the
literary-wren-cfeb4b
branch
from
67130ba to
0a5c856
Compare
Kitenite
force-pushed
the
literary-wren-cfeb4b
branch
from
0a5c856 to
5955504
Compare
Kitenite
force-pushed
the
literary-wren-cfeb4b
branch
from
5955504 to
fb9aec6
Compare
Kitenite
force-pushed
the
literary-wren-cfeb4b
branch
from
fb9aec6 to
745277e
Compare
When creating a new workspace with setup commands, the terminal tab is now automatically named "Workspace Setup" instead of the default "Terminal N" naming. Also removes the unused tabTitle parameter from the terminal createOrAttach API since tab naming is handled at the UI layer via setTabAutoTitle. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Kitenite
force-pushed
the
literary-wren-cfeb4b
branch
from
745277e to
7a9e57b
Compare
![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
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)
42-42: Critical: Environment variable mismatch causes script to always exit.Line 42 checks for
SUPERSET_TAB_ID, but the environment variable was renamed toSUPERSET_PANE_ID(as used on line 65 and set in terminal-manager.ts line 139). This means the notification hook will never execute because the guard condition will always fail.Apply this diff to fix the variable name:
-[ -z "$SUPERSET_TAB_ID" ] && exit 0 +[ -z "$SUPERSET_PANE_ID" ] && exit 0
🧹 Nitpick comments (2)
apps/desktop/src/main/lib/terminal-manager.ts (1)
47-66: API simplification looks good, but parameter naming could be clearer.The removal of
tabTitle,workspaceName, androotPathparameters simplifies the API. However, the parameter is still namedtabIdwhen it semantically represents apaneId(as noted in the comment on line 139). This naming inconsistency could confuse future maintainers.Consider renaming the
tabIdparameter topaneIdthroughout this file for clarity:-async createOrAttach(params: { - tabId: string; +async createOrAttach(params: { + paneId: string; workspaceId: string;And update all references accordingly. This would eliminate the need for the explanatory comment on line 139.
apps/desktop/src/main/windows/main.ts (1)
85-105: Consider extracting the data derivation logic for better maintainability.The inline derivation of
workspaceNameandtitlefrom runtime data works correctly but has a complex type assertion (lines 97-99) that's difficult to read and maintain. The logic also depends on the store's internal structure, which could change.Consider extracting this logic into a helper function:
function deriveNotificationMetadata( workspaceId: string, paneId: string ): { workspaceName: string; title: string } { // Derive workspace name const workspace = db.data.workspaces.find((w) => w.id === workspaceId); const worktree = workspace ? db.data.worktrees.find((wt) => wt.id === workspace.worktreeId) : undefined; const workspaceName = workspace?.name || worktree?.branch || "Workspace"; // Derive title from pane/tab const tabsStorage = store.get("tabs-storage") as TabsStorage | undefined; const pane = tabsStorage?.state?.panes?.[paneId]; const tab = pane ? tabsStorage?.state?.tabs?.find((t) => t.id === pane.tabId) : undefined; const title = pane?.name || tab?.userTitle?.trim() || tab?.name || "Terminal"; return { workspaceName, title }; }This would improve readability and make the type assertion easier to manage by defining a proper
TabsStorageinterface.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/desktop/src/lib/trpc/routers/notifications.ts(2 hunks)apps/desktop/src/lib/trpc/routers/terminal/terminal.ts(2 hunks)apps/desktop/src/main/lib/agent-setup.ts(1 hunks)apps/desktop/src/main/lib/notifications/server.ts(2 hunks)apps/desktop/src/main/lib/terminal-manager.test.ts(0 hunks)apps/desktop/src/main/lib/terminal-manager.ts(1 hunks)apps/desktop/src/main/windows/main.ts(3 hunks)apps/desktop/src/renderer/react-query/workspaces/useCreateWorkspace.ts(2 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx(0 hunks)apps/desktop/src/renderer/stores/tabs/useAgentHookListener.ts(2 hunks)
💤 Files with no reviewable changes (2)
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
- apps/desktop/src/main/lib/terminal-manager.test.ts
🧰 Additional context used
📓 Path-based instructions (6)
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/main/lib/agent-setup.tsapps/desktop/src/main/lib/notifications/server.tsapps/desktop/src/renderer/stores/tabs/useAgentHookListener.tsapps/desktop/src/main/windows/main.tsapps/desktop/src/lib/trpc/routers/notifications.tsapps/desktop/src/main/lib/terminal-manager.tsapps/desktop/src/lib/trpc/routers/terminal/terminal.tsapps/desktop/src/renderer/react-query/workspaces/useCreateWorkspace.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/main/lib/agent-setup.tsapps/desktop/src/main/lib/notifications/server.tsapps/desktop/src/renderer/stores/tabs/useAgentHookListener.tsapps/desktop/src/main/windows/main.tsapps/desktop/src/lib/trpc/routers/notifications.tsapps/desktop/src/main/lib/terminal-manager.tsapps/desktop/src/lib/trpc/routers/terminal/terminal.tsapps/desktop/src/renderer/react-query/workspaces/useCreateWorkspace.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Maintain type safety and avoid using
anyunless absolutely necessary in TypeScript code
Files:
apps/desktop/src/main/lib/agent-setup.tsapps/desktop/src/main/lib/notifications/server.tsapps/desktop/src/renderer/stores/tabs/useAgentHookListener.tsapps/desktop/src/main/windows/main.tsapps/desktop/src/lib/trpc/routers/notifications.tsapps/desktop/src/main/lib/terminal-manager.tsapps/desktop/src/lib/trpc/routers/terminal/terminal.tsapps/desktop/src/renderer/react-query/workspaces/useCreateWorkspace.ts
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in renderer process or shared code - only in src/main/
Files:
apps/desktop/src/renderer/stores/tabs/useAgentHookListener.tsapps/desktop/src/renderer/react-query/workspaces/useCreateWorkspace.ts
apps/desktop/src/lib/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules like node:fs, node:path, node:os in src/lib/electron-router-dom.ts or similar shared code
Files:
apps/desktop/src/lib/trpc/routers/notifications.tsapps/desktop/src/lib/trpc/routers/terminal/terminal.ts
apps/desktop/src/main/lib/terminal-*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use node-pty for terminal session management in the desktop app
Files:
apps/desktop/src/main/lib/terminal-manager.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/main/lib/terminal-*.ts : Use node-pty for terminal session management in the desktop app
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/main/lib/*{workspace,worktree}-*.ts : Use git worktree-based workspace management in the desktop app through workspace-manager.ts and worktree-manager.ts
Applied to files:
apps/desktop/src/main/windows/main.tsapps/desktop/src/main/lib/terminal-manager.tsapps/desktop/src/lib/trpc/routers/terminal/terminal.tsapps/desktop/src/renderer/react-query/workspaces/useCreateWorkspace.ts
📚 Learning: 2025-11-24T21:33:13.267Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
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/main/windows/main.tsapps/desktop/src/renderer/react-query/workspaces/useCreateWorkspace.ts
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/shared/ipc-channels.ts : Define IPC channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers
Applied to files:
apps/desktop/src/main/windows/main.ts
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/main/lib/*-ipcs.ts : Use type-safe IPC system with handlers accepting single object parameters, not positional parameters
Applied to files:
apps/desktop/src/main/windows/main.ts
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/main/lib/terminal-*.ts : Use node-pty for terminal session management in the desktop app
Applied to files:
apps/desktop/src/main/lib/terminal-manager.tsapps/desktop/src/lib/trpc/routers/terminal/terminal.tsapps/desktop/src/renderer/react-query/workspaces/useCreateWorkspace.ts
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/main/index.ts : Load environment variables from monorepo root .env file in src/main/index.ts with override: true
Applied to files:
apps/desktop/src/main/lib/terminal-manager.ts
📚 Learning: 2025-11-24T21:33:13.267Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Please use alias as defined in `tsconfig.json` when possible
Applied to files:
apps/desktop/src/lib/trpc/routers/terminal/terminal.ts
🧬 Code graph analysis (2)
apps/desktop/src/lib/trpc/routers/terminal/terminal.ts (1)
apps/desktop/src/main/lib/db/index.ts (1)
db(18-25)
apps/desktop/src/renderer/react-query/workspaces/useCreateWorkspace.ts (1)
apps/desktop/src/renderer/stores/tabs/store.ts (1)
useTabsStore(72-615)
⏰ 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 (13)
apps/desktop/src/main/lib/agent-setup.ts (1)
64-68: LGTM! Notification payload correctly updated to use paneId.The curl command now sends
paneIdinstead of the removedtabId,tabTitle, andworkspaceNameparameters, which aligns with the server-side changes in notifications/server.ts.apps/desktop/src/lib/trpc/routers/notifications.ts (2)
8-10: LGTM! Type definitions correctly updated to use paneId.The NotificationEvent union type now uses
paneIdinstead oftabIdfor the focus-tab variant, maintaining type safety and consistency with the broader refactoring.
23-25: LGTM! Event handler signature updated consistently.The onFocusTab handler parameter correctly reflects the updated data shape using
paneId.apps/desktop/src/main/lib/terminal-manager.ts (1)
136-142: LGTM! Environment variables correctly updated to pane-based identifiers.The environment setup now uses
SUPERSET_PANE_IDinstead of the legacy tab-related variables, which aligns with the broader refactoring. The simplified environment reduces complexity.apps/desktop/src/renderer/stores/tabs/useAgentHookListener.ts (2)
16-32: LGTM! Event data handling correctly updated to use paneId.The destructuring of
paneIdfromevent.dataaligns with the updated notification payload structure. The rest of the logic correctly usespaneIdfor pane lookups.
33-66: LGTM! Focus-tab event handling properly updated.The focus-tab handler correctly destructures and uses
paneIdfrom the event data, maintaining consistency with the updated notification schema.apps/desktop/src/main/windows/main.ts (1)
107-130: LGTM! Notification behavior correctly implements dynamic content and paneId-based focus.The notification title and body now use derived context (workspaceName and title), and the click handler correctly emits
paneIdfor focus-tab events. This aligns with the PR objectives.apps/desktop/src/renderer/react-query/workspaces/useCreateWorkspace.ts (1)
33-41: LGTM! Feature implementation correctly sets auto-title for workspace setup terminals.The code properly:
- Destructures both
tabIdandpaneIdfromaddTab(matching the store's return type)- Sets the auto-title to "Workspace Setup" using the new
setTabAutoTitlemethod- Uses
paneIdas thetabIdparameter increateOrAttach.mutate, which aligns with terminal-manager's current parameter semanticsThis successfully implements the PR objective of auto-naming setup workspace terminals.
apps/desktop/src/lib/trpc/routers/terminal/terminal.ts (3)
13-18: LGTM! Documentation accurately reflects the updated environment variables.The comment correctly documents the new environment variable set:
PATH,SUPERSET_PANE_ID,SUPERSET_WORKSPACE_ID, andSUPERSET_PORT, which matches the implementation in terminal-manager.ts.
22-30: LGTM! Input schema simplified by removing tabTitle parameter.The removal of
tabTitlefrom the input schema aligns with the broader refactoring to handle tab naming at the UI layer viasetTabAutoTitleinstead of passing it through the terminal API.
43-56: LGTM! Terminal creation simplified with pane-based identifiers.The workspace lookup now focuses solely on resolving
cwd, and thecreateOrAttachcall correctly passes the simplified parameter set withouttabTitle,workspaceName, orrootPath.apps/desktop/src/main/lib/notifications/server.ts (2)
4-8: LGTM! AgentCompleteEvent interface correctly updated to use paneId.The interface now uses
paneIdinstead of the removedtabId,tabTitle, andworkspaceNamefields, simplifying the event payload and aligning with the pane-based naming convention.
25-41: LGTM! Request handler correctly processes paneId parameter.The endpoint properly:
- Reads
paneIdfrom query parameters- Validates
paneIdpresence and type- Constructs the event with
paneId- Returns
paneIdin the responseThis aligns with the agent-setup.ts curl command that sends
paneIdin the request.
…s to notifications - Sessions are now keyed by paneId (panes are children of tabs) - Terminal creates pass both paneId and tabId to allow proper hierarchy tracking - Notification hooks now receive paneId, tabId, and workspaceId - Notification titles derived from pane name first, then tab userTitle/name - Updated all terminal router methods to use paneId consistently - Updated tests to reflect the paneId-based session keying 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The dev Claude wrapper was calling the prod wrapper instead of the real binary because findRealBinary only filtered its own bin dir. Now filters both .superset and .superset-dev bin directories using SUPERSET_DIR_NAMES constant. Also removes debug logging and adds guards for undefined paneId/workspaceId. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add TabsState schema to lowdb (Pane, UITab, TabsState) - Add tabs-state:get/set IPC handlers using lowdb - Create lowdb storage adapter for zustand persist - Update tabs store to use lowdb instead of electron-store - Simplify main.ts notification handler to read from db.data.tabsState 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Create dedicated lowdb instance for UI state (app-state.json): - Add app-state/ module with schemas and initialization - Move TabsState out of db/schemas.ts into app-state/schemas.ts - Add APP_STATE_PATH to app-environment.ts - Update storage-ipcs.ts to use appState - Initialize appState in main/index.ts This keeps domain data (db.json) separate from UI state (app-state.json) while using lowdb for both, providing consistent API without JSON parsing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Kitenite
changed the title
Merge loaded data with defaults to prevent crashes when accessing tabsState.panes on legacy files that use different key names. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use optional chaining when accessing tabsState.panes and tabsState.tabs to prevent crashes if tabsState is undefined in legacy app-state.json. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/desktop/src/lib/trpc/routers/terminal/terminal.ts(11 hunks)apps/desktop/src/main/lib/app-state/index.ts(1 hunks)apps/desktop/src/main/lib/terminal-manager.test.ts(45 hunks)apps/desktop/src/main/lib/terminal-manager.ts(18 hunks)apps/desktop/src/main/windows/main.ts(3 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/main/lib/app-state/index.ts
🧰 Additional context used
📓 Path-based instructions (8)
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/main/windows/main.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/lib/trpc/routers/terminal/terminal.tsapps/desktop/src/main/lib/terminal-manager.test.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/main/windows/main.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/lib/trpc/routers/terminal/terminal.tsapps/desktop/src/main/lib/terminal-manager.test.tsapps/desktop/src/main/lib/terminal-manager.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Maintain type safety and avoid using
anyunless absolutely necessary in TypeScript code
Files:
apps/desktop/src/main/windows/main.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/lib/trpc/routers/terminal/terminal.tsapps/desktop/src/main/lib/terminal-manager.test.tsapps/desktop/src/main/lib/terminal-manager.ts
**/components/**/[A-Z]*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use one folder per component with structure ComponentName/ComponentName.tsx + index.ts barrel export
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in renderer process or shared code - only in src/main/
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
apps/desktop/src/lib/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules like node:fs, node:path, node:os in src/lib/electron-router-dom.ts or similar shared code
Files:
apps/desktop/src/lib/trpc/routers/terminal/terminal.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/main/lib/terminal-manager.test.ts
apps/desktop/src/main/lib/terminal-*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use node-pty for terminal session management in the desktop app
Files:
apps/desktop/src/main/lib/terminal-manager.test.tsapps/desktop/src/main/lib/terminal-manager.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
Learning: Applies to apps/desktop/**/*.{ts,tsx,js,jsx} : For Electron interprocess communication, ALWAYS use tRPC as defined in `src/lib/trpc`
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/main/lib/terminal-*.ts : Use node-pty for terminal session management in the desktop app
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/main/lib/*{workspace,worktree}-*.ts : Use git worktree-based workspace management in the desktop app through workspace-manager.ts and worktree-manager.ts
Applied to files:
apps/desktop/src/main/windows/main.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/lib/trpc/routers/terminal/terminal.tsapps/desktop/src/main/lib/terminal-manager.test.tsapps/desktop/src/main/lib/terminal-manager.ts
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/main/lib/terminal-*.ts : Use node-pty for terminal session management in the desktop app
Applied to files:
apps/desktop/src/main/windows/main.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/lib/trpc/routers/terminal/terminal.tsapps/desktop/src/main/lib/terminal-manager.test.tsapps/desktop/src/main/lib/terminal-manager.ts
📚 Learning: 2025-11-24T21:33:13.267Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
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/main/windows/main.ts
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/shared/ipc-channels.ts : Define IPC channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers
Applied to files:
apps/desktop/src/main/windows/main.ts
📚 Learning: 2025-11-24T21:33:13.267Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
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/Terminal/Terminal.tsx
📚 Learning: 2025-11-24T21:33:13.267Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
Learning: Applies to apps/desktop/**/*.test.{ts,tsx,js,jsx} : Tests should be independent
Applied to files:
apps/desktop/src/main/lib/terminal-manager.test.ts
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/main/index.ts : Load environment variables from monorepo root .env file in src/main/index.ts with override: true
Applied to files:
apps/desktop/src/main/lib/terminal-manager.ts
🧬 Code graph analysis (3)
apps/desktop/src/main/windows/main.ts (2)
apps/desktop/src/main/lib/app-state/index.ts (1)
appState(39-46)apps/desktop/src/main/lib/notifications/server.ts (1)
notificationsEmitter(11-11)
apps/desktop/src/main/lib/terminal-manager.test.ts (1)
apps/desktop/src/main/lib/terminal-history.ts (1)
getHistoryDir(15-20)
apps/desktop/src/main/lib/terminal-manager.ts (1)
apps/desktop/src/main/lib/terminal-history.ts (1)
HistoryReader(107-143)
⏰ 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 (7)
apps/desktop/src/main/windows/main.ts (3)
9-11: LGTM! Imports align with tRPC storage migration.These imports support the PR's goal of centralizing state in Lowdb-backed appState and deriving notification context at runtime.
105-113: LGTM! Notification construction is clear and correct.The notification title and body correctly use the derived workspace and tab/pane context, with appropriate fallback values.
117-126: LGTM! Pane-centric focus aligns with PR objectives.The addition of
paneIdto the focus-tab event correctly implements the shift from tab-centric to pane-centric identifiers mentioned in the PR summary.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx (1)
204-215: Pane‑centric terminal wiring looks consistent end‑to‑end.All manager‑facing calls (createOrAttach, write, resize, clearScrollback, detach, drag‑drop writes) now correctly use
paneIdas the session key while still passingtabIdonly as metadata for titles/focus. The unmount path detaching instead of killing aligns with the “reattach to same PTY” behavior and should work well with the new paneId‑based manager and stream subscription.Please make sure terminal interactions still behave correctly under React Strict Mode by running the existing terminal integration tests (including any renderer E2E/CT that cover Terminal panes).
Also applies to: 229-235, 264-273, 296-305, 315-317, 335-337, 367-369
apps/desktop/src/main/lib/terminal-manager.test.ts (1)
81-89: Tests now accurately reflect paneId‑based session identity and history.The test updates consistently use
paneIdfor session creation, event names (data:pane-*/exit:pane-*), and history paths, giving good coverage of the new identity model (including recovery, killByWorkspaceId, and multi‑session persistence). The behavior under the paneId scheme is well exercised.Please run the
TerminalManagertest suite (e.g., via your existingbun testtask) to confirm everything passes on your environment, especially the history and killByWorkspaceId scenarios which depend on timing.Also applies to: 105-120, 128-152, 158-165, 170-192, 196-207, 219-223, 238-260, 269-272, 281-303, 308-349, 354-370, 381-388, 394-402, 410-417, 434-467, 464-492, 500-505, 519-525, 541-544, 548-552, 567-576, 580-590, 601-605, 618-622, 632-635, 657-671, 684-693, 711-715, 723-727, 736-739, 747-748, 772-774, 785-788, 799-802, 813-817, 843-847, 873-876
apps/desktop/src/lib/trpc/routers/terminal/terminal.ts (1)
11-19: tRPC terminal router is cleanly migrated to paneId and aligned with TerminalManager.The router’s inputs/outputs now consistently use
paneIdas the session key (including createOrAttach, write/resize/signal/kill/detach/clearScrollback, getSession, and stream), while still resolving cwd and passingtabIdonly where needed. Stream subscriptions correctly listen to and clean updata:${paneId}/exit:${paneId}handlers, matching the updated TerminalManager API.Given the public API shift, please ensure all downstream callers (renderer, workers, tests) compile and type‑check against these new
paneId‑centric procedure signatures.Also applies to: 25-32, 36-43, 46-47, 52-60, 63-67, 73-75, 84-88, 97-99, 108-110, 122-123, 136-137, 145-147, 153-165, 168-169, 183-190
apps/desktop/src/main/lib/terminal-manager.ts (1)
14-15: PaneId‑based session identity and Strict‑Mode‑safe creation logic look solid, with only minor naming nits.
- Using
paneIdas the sole key forsessions,pendingSessions, HistoryReader/Writer, and data/exit event channels ensures consistent identity across manager, router, and renderer, and aligns with history paths exercised in tests.- The
pendingSessionsmap and “deduplicate concurrent calls for the same paneId” logic correctly guard against duplicate PTY creation in React Strict Mode; thefinallycleanup avoids leaks even on failure.- Emitting filtered data to history but raw data on
data:${paneId}fixes protocol/escape‑sequence handling without regressing scrollback correctness.- Only minor nit: in
killByWorkspaceIdandcleanupthe loop variabletabIdnow representspaneId; renaming tosessionId/paneIdin a follow‑up would improve readability but isn’t functionally blocking.Please rerun the terminal manager tests and any higher‑level terminal integration tests to validate the new deduplication behavior under concurrent
createOrAttachcalls and confirm that paneId‑based history recovery still behaves as expected.
[scratchpad_end] -->Also applies to: 50-58, 63-68, 72-73, 76-77, 86-95, 97-97, 101-102, 106-114, 120-128, 140-144, 153-155, 179-185, 189-201, 208-221, 233-245, 247-253, 261-267, 273-293, 295-303, 310-331, 334-344, 346-361, 367-373, 377-383, 406-423, 432-453, 462-473, 523-528, 535-539, 566-568
Rename the tabId parameter to paneId in terminal-history.ts and terminal-manager.ts loop variables to match the session key naming convention used throughout the codebase. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ndler
Wrap db.data and appState.data access in try-catch blocks to prevent
crashes if proxies throw or data has unexpected shape. Falls back to
safe defaults ("Workspace" and "Terminal") on error.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…inal Re-add environment variables that were accidentally removed in #303: - SUPERSET_WORKSPACE_NAME: The workspace name - SUPERSET_WORKSPACE_PATH: The worktree path - SUPERSET_ROOT_PATH: The main repo path These are needed by setup/teardown scripts that run in terminal sessions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…inal (#311) Re-add environment variables that were accidentally removed in #303: - SUPERSET_WORKSPACE_NAME: The workspace name - SUPERSET_WORKSPACE_PATH: The worktree path - SUPERSET_ROOT_PATH: The main repo path These are needed by setup/teardown scripts that run in terminal sessions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 participant
Summary
app-state.jsonfileChanges
uiStaterouter withtabs.get/setandtheme.get/setprocedurestrpc-storageadapter for Zustand persist middlewareappState.update()bug (lowdb's JSONFilePreset doesn't have update method)layoutfield to tab Zod schema to preserve MosaicNode layoutsTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.