feat(desktop): add static port configuration via ports.json#683
feat(desktop): add static port configuration via ports.json#683Kitenite merged 9 commits intosuperset-sh:mainfrom
Conversation
Users can now define static ports in a `.superset/ports.json` file within their workspace. When present, static ports replace dynamic port discovery in the sidebar. Features: - New static-ports module with loader and file watcher - tRPC procedures: hasStaticConfig, getStatic, subscribeStatic - Auto-reload when ports.json changes - Toast notifications for config errors - Marketing documentation page at /ports 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Warning Rate limit exceeded@Kitenite has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 18 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 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 (4)
📝 WalkthroughWalkthroughAdds static ports via a Changes
Sequence Diagram(s)sequenceDiagram
participant UI as PortsList UI
participant Store as Renderer Store
participant tRPC as tRPC Router
participant Loader as Static Ports Loader
participant FS as File System
participant Watcher as StaticPortsWatcher
UI->>tRPC: hasStaticConfig(workspaceId)
tRPC->>Loader: hasStaticPortsConfig(worktreePath)
Loader->>FS: Stat .superset/ports.json
FS-->>Loader: exists?/mtime
Loader-->>tRPC: { hasStatic }
tRPC-->>UI: hasStatic result
alt Static Config Available
UI->>tRPC: getStatic(workspaceId)
tRPC->>Loader: loadStaticPorts(worktreePath)
Loader->>FS: Read & parse ports.json
Loader->>Loader: Validate schema & entries
Loader-->>tRPC: { ports | error }
tRPC-->>Store: setStaticPorts / setStaticPortsError
Store-->>UI: render merged ports via mergePorts
UI->>tRPC: subscribeStatic(workspaceId)
tRPC->>Watcher: watch(workspaceId, worktreePath)
Watcher->>FS: monitor .superset/ports.json
FS-->>Watcher: file change event
Watcher->>tRPC: emit change for workspaceId
tRPC-->>UI: notify to reload
UI->>tRPC: getStatic(workspaceId) (reload)
else No Static Config
UI->>tRPC: fallback to dynamic ports flow
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 2
🤖 Fix all issues with AI agents
In @apps/desktop/plans/20260108-2251-static-ports-json.md:
- Around line 122-124: Update the "Toast Notifications" section to use the
correct import path: replace the reference to importing from "sonner" with
importing from "@superset/ui/sonner" and show using toast (e.g.,
toast.error("message")); ensure the section aligns with the "Surprises &
Discoveries" note and mentions the symbol "toast" imported from
"@superset/ui/sonner" instead of "sonner".
- Around line 200-217: Remove or strike through the "Milestone 4: Renderer Store
Updates" section that proposes adding staticPorts, staticPortsError,
useStaticPorts and related actions (setStaticPorts, setStaticPortsError,
setUseStaticPorts, clearStaticPorts) since the Decision Log indicates the
implementation uses tRPC queries directly and the ports store was not changed;
update the markdown to reflect that the store changes were intentionally skipped
to avoid confusion.
🧹 Nitpick comments (6)
apps/desktop/src/shared/types/ports.ts (1)
11-21: Consider defining a separate public type for clarity.The types are functionally correct. However, using
Omit<StaticPort, "workspaceId">for the public API adds cognitive overhead. Consider defining a separatePublicStaticPorttype to make the distinction explicit:export interface PublicStaticPort { port: number; label: string; } export interface StaticPort extends PublicStaticPort { workspaceId: string; } export interface StaticPortsResult { exists: boolean; ports: PublicStaticPort[] | null; error: string | null; }This approach makes the public vs internal distinction clearer and is easier to understand at a glance. The current
Omitapproach works but requires readers to mentally compute the resulting type.♻️ Alternative type structure
+export interface PublicStaticPort { + port: number; + label: string; +} + -export interface StaticPort { +export interface StaticPort extends PublicStaticPort { - port: number; - label: string; workspaceId: string; } export interface StaticPortsResult { exists: boolean; - ports: Omit<StaticPort, "workspaceId">[] | null; + ports: PublicStaticPort[] | null; error: string | null; }apps/desktop/src/main/lib/static-ports/loader.test.ts (1)
6-9: Consider extracting magic strings to constants.The test constants reference
.supersetandports.jsonas strings. While this works, consider importingPROJECT_SUPERSET_DIR_NAMEandPORTS_FILE_NAMEfrom@/shared/constantsto ensure consistency with the implementation and avoid duplication.♻️ Use shared constants
+import { PROJECT_SUPERSET_DIR_NAME, PORTS_FILE_NAME } from "@/shared/constants"; import { join } from "node:path"; import { hasStaticPortsConfig, loadStaticPorts } from "./loader"; const TEST_DIR = join(__dirname, ".test-tmp"); const WORKTREE_PATH = join(TEST_DIR, "worktree"); -const SUPERSET_DIR = join(WORKTREE_PATH, ".superset"); -const PORTS_FILE = join(SUPERSET_DIR, "ports.json"); +const SUPERSET_DIR = join(WORKTREE_PATH, PROJECT_SUPERSET_DIR_NAME); +const PORTS_FILE = join(SUPERSET_DIR, PORTS_FILE_NAME);apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/PortsList.tsx (1)
326-353: Consider extractingStaticPortBadgeto its own file.Per coding guidelines, each file should contain one component.
StaticPortBadgeis a distinct, reusable component that could be extracted to maintain consistency with project conventions.That said, given it's tightly coupled to
PortsListand relatively small, this is a minor concern that could be addressed in a follow-up.apps/desktop/src/main/lib/static-ports/loader.ts (1)
128-139: Consider validating for duplicate port numbers.If a user accidentally defines the same port twice in their
ports.json, both entries will be loaded. This could cause confusion in the UI. Consider adding a check for duplicate ports and returning an appropriate error.♻️ Proposed fix to detect duplicate ports
const validatedPorts: Array<{ port: number; label: string }> = []; + const seenPorts = new Set<number>(); for (let i = 0; i < parsed.ports.length; i++) { const entry = parsed.ports[i] as PortEntry; const result = validatePortEntry(entry, i); if (!result.valid) { return { exists: true, ports: null, error: result.error }; } + if (seenPorts.has(result.port)) { + return { + exists: true, + ports: null, + error: `ports[${i}].port ${result.port} is a duplicate`, + }; + } + seenPorts.add(result.port); + validatedPorts.push({ port: result.port, label: result.label }); }apps/desktop/src/main/lib/static-ports/watcher.ts (1)
103-112: Extract debounce delay to a named constant.Per coding guidelines, magic numbers should be extracted to named constants at module top.
♻️ Proposed fix
+const DEBOUNCE_DELAY_MS = 100; + /** * Watches for changes to ports.json files across workspaces.Then at line 112:
- }, 100); + }, DEBOUNCE_DELAY_MS);apps/desktop/src/lib/trpc/routers/ports/ports.ts (1)
52-56: Consider extracting workspace lookup to a helper.The pattern of looking up workspace by ID and getting its path is repeated in all three new procedures. A helper function would reduce duplication.
♻️ Proposed refactor
Add a helper at the top of the file or in a utils module:
function getWorkspacePathById(workspaceId: string): string | null { const workspace = localDb .select() .from(workspaces) .where(eq(workspaces.id, workspaceId)) .get(); if (!workspace) return null; return getWorkspacePath(workspace); }Then simplify each procedure:
hasStaticConfig: publicProcedure .input(z.object({ workspaceId: z.string() })) .query(({ input }): { hasStatic: boolean } => { - const workspace = localDb - .select() - .from(workspaces) - .where(eq(workspaces.id, input.workspaceId)) - .get(); - - if (!workspace) { - return { hasStatic: false }; - } - - const workspacePath = getWorkspacePath(workspace); + const workspacePath = getWorkspacePathById(input.workspaceId); if (!workspacePath) { return { hasStatic: false }; } return { hasStatic: hasStaticPortsConfig(workspacePath) }; }),Also applies to: 75-79, 116-120
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/desktop/plans/20260108-2251-static-ports-json.mdapps/desktop/src/lib/trpc/routers/ports/ports.tsapps/desktop/src/main/lib/static-ports/index.tsapps/desktop/src/main/lib/static-ports/loader.test.tsapps/desktop/src/main/lib/static-ports/loader.tsapps/desktop/src/main/lib/static-ports/watcher.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/PortsList.tsxapps/desktop/src/shared/constants.tsapps/desktop/src/shared/types/ports.tsapps/marketing/src/app/ports/page.tsx
🧰 Additional context used
📓 Path-based instructions (7)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/shared/types/ports.tsapps/desktop/src/shared/constants.tsapps/desktop/src/main/lib/static-ports/loader.tsapps/desktop/src/main/lib/static-ports/watcher.tsapps/desktop/src/lib/trpc/routers/ports/ports.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/PortsList.tsxapps/desktop/src/main/lib/static-ports/loader.test.tsapps/desktop/src/main/lib/static-ports/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use object parameters for functions with 2+ parameters instead of positional arguments
Functions with 2+ parameters should accept a single params object with named properties for self-documentation and extensibility
Use prefixed console logging with context pattern: [domain/operation] message
Extract magic numbers and hardcoded values to named constants at module top
Use lookup objects/maps instead of repeated if (type === ...) conditionals
Avoid usinganytype - maintain type safety in TypeScript code
Never swallow errors silently - at minimum log them with context
Import from concrete files directly when possible - avoid barrel file abuse that creates circular dependencies
Avoid deep nesting (4+ levels) - use early returns, extract functions, and invert conditions
Use named properties in options objects instead of boolean parameters to avoid boolean blindness
Files:
apps/desktop/src/shared/types/ports.tsapps/desktop/src/shared/constants.tsapps/marketing/src/app/ports/page.tsxapps/desktop/src/main/lib/static-ports/loader.tsapps/desktop/src/main/lib/static-ports/watcher.tsapps/desktop/src/lib/trpc/routers/ports/ports.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/PortsList.tsxapps/desktop/src/main/lib/static-ports/loader.test.tsapps/desktop/src/main/lib/static-ports/index.ts
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations - never use raw SQL
Files:
apps/desktop/src/shared/types/ports.tsapps/desktop/src/shared/constants.tsapps/marketing/src/app/ports/page.tsxapps/desktop/src/main/lib/static-ports/loader.tsapps/desktop/src/main/lib/static-ports/watcher.tsapps/desktop/src/lib/trpc/routers/ports/ports.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/PortsList.tsxapps/desktop/src/main/lib/static-ports/loader.test.tsapps/desktop/src/main/lib/static-ports/index.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for formatting and linting - run at root level with
bun run lint:fixorbiome check --write
Files:
apps/desktop/src/shared/types/ports.tsapps/desktop/src/shared/constants.tsapps/marketing/src/app/ports/page.tsxapps/desktop/src/main/lib/static-ports/loader.tsapps/desktop/src/main/lib/static-ports/watcher.tsapps/desktop/src/lib/trpc/routers/ports/ports.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/PortsList.tsxapps/desktop/src/main/lib/static-ports/loader.test.tsapps/desktop/src/main/lib/static-ports/index.ts
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
One component per file - do not create multi-component files
Files:
apps/marketing/src/app/ports/page.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/PortsList.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/PortsList.tsx
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Co-locate tests with implementation files using .test.ts or .test.tsx suffix
Files:
apps/desktop/src/main/lib/static-ports/loader.test.ts
🧠 Learnings (2)
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/desktop/src/renderer/**/*.{ts,tsx} : Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility
Applied to files:
apps/desktop/src/main/lib/static-ports/index.ts
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/desktop/src/lib/electron-router-dom.ts : Do not import Node.js modules like node:path or dotenv in electron-router-dom.ts and similar shared files - they run in both main and renderer processes
Applied to files:
apps/desktop/src/main/lib/static-ports/index.ts
🧬 Code graph analysis (4)
apps/desktop/src/main/lib/static-ports/loader.ts (3)
apps/desktop/src/main/lib/static-ports/index.ts (2)
loadStaticPorts(1-1)hasStaticPortsConfig(1-1)apps/desktop/src/shared/types/ports.ts (1)
StaticPortsResult(17-21)apps/desktop/src/shared/constants.ts (2)
PROJECT_SUPERSET_DIR_NAME(33-33)PORTS_FILE_NAME(36-36)
apps/desktop/src/lib/trpc/routers/ports/ports.ts (4)
apps/desktop/src/lib/trpc/routers/workspaces/utils/worktree.ts (1)
getWorkspacePath(22-43)apps/desktop/src/main/lib/static-ports/loader.ts (2)
hasStaticPortsConfig(150-157)loadStaticPorts(69-142)apps/desktop/src/shared/types/ports.ts (1)
StaticPort(11-15)apps/desktop/src/main/lib/static-ports/watcher.ts (1)
staticPortsWatcher(155-155)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/PortsList.tsx (4)
packages/ui/src/components/ui/sonner.tsx (1)
toast(40-40)apps/desktop/src/shared/types/ports.ts (1)
StaticPort(11-15)packages/ui/src/components/ui/tooltip.tsx (3)
Tooltip(76-76)TooltipTrigger(76-76)TooltipContent(76-76)apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/constants.ts (1)
STROKE_WIDTH(2-2)
apps/desktop/src/main/lib/static-ports/loader.test.ts (1)
apps/desktop/src/main/lib/static-ports/loader.ts (2)
loadStaticPorts(69-142)hasStaticPortsConfig(150-157)
🪛 markdownlint-cli2 (0.18.1)
apps/desktop/plans/20260108-2251-static-ports-json.md
257-257: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
🔇 Additional comments (16)
apps/desktop/src/shared/constants.ts (1)
36-36: LGTM! Constant follows established patterns.The constant is properly named, well-placed near related configuration constants, and follows the existing conventions in this file.
apps/desktop/src/main/lib/static-ports/index.ts (1)
1-2: LGTM! Appropriate use of barrel exports for public API.This barrel export file provides a clean, controlled public API surface for the static-ports module. While the coding guidelines generally discourage barrel files, this is an appropriate exception as it:
- Defines a clear module boundary
- Exposes only the intended public functions
- Shows no risk of circular dependencies (loader and watcher are independent)
apps/marketing/src/app/ports/page.tsx (2)
1-237: LGTM! Documentation is accurate and well-structured.The marketing page provides clear, comprehensive documentation that accurately reflects the implementation. The content is well-organized into logical sections, and all technical details match the actual behavior defined in the loader and types.
Key strengths:
- Accurate schema documentation matching
StaticPorttype- Correct file paths and validation rules
- Clear explanation of workspace-scoped behavior
- Appropriate use of code examples
156-156: Toast notification implementation already exists in the codebase.The error toast for malformed
ports.jsonis fully implemented inapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/PortsList.tsx. When static ports fail to load, the component displays an error toast withtoast.error("Failed to load ports.json", { description: ... })and prevents duplicate notifications using a ref tracker. This matches and validates the documentation claim at line 156 of the marketing page.Likely an incorrect or invalid review comment.
apps/desktop/src/main/lib/static-ports/loader.test.ts (1)
1-285: LGTM! Comprehensive test coverage with excellent quality.The test suite demonstrates exceptional thoroughness:
Coverage highlights:
- ✅ All success paths (missing file, valid configs, edge cases)
- ✅ All validation error paths (JSON syntax, schema, types, ranges)
- ✅ Boundary testing (ports 1 and 65535, empty arrays, whitespace)
- ✅ Error message specificity (including array indices)
- ✅ Both public functions (
loadStaticPortsandhasStaticPortsConfig)Test quality:
- Clear, descriptive test names
- Proper test isolation with setup/teardown
- Specific assertions on result structure
- Covers both positive and negative cases
- Matches loader implementation validation rules exactly
The 24 passing tests provide strong confidence in the loader's correctness.
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/PortsList.tsx (3)
31-59: LGTM! Clean tRPC integration for static ports.The static ports data flow is well-structured:
hasStaticConfigquery gates the featuregetStaticquery fetches ports data when enabledsubscribeStaticsubscription handles live reload by invalidating queriesThe subscription being always enabled (when workspace exists) is appropriate since it needs to detect file creation, not just changes.
61-77: Good error deduplication pattern.Using a ref to track the last error prevents duplicate toasts on re-renders while still showing new errors. The cleanup when error clears is also handled correctly.
186-198: Clean conditional rendering between static and dynamic modes.The ternary clearly separates the two rendering paths. Static ports render as a flat list while dynamic ports maintain workspace grouping. This aligns with the design decision that static ports are workspace-scoped.
apps/desktop/src/main/lib/static-ports/loader.ts (2)
15-61: Thorough port entry validation.The validation function handles all edge cases well:
- Type checks before accessing properties
- Explicit field presence checks
- Integer validation for port numbers
- Valid port range (1-65535)
- Non-empty string validation for labels
- Label trimming for cleaner output
The indexed error messages (
ports[${index}]) are helpful for users to locate issues in their config.
69-142: Clean loader implementation with proper error handling.The loader follows a clear pattern:
- Check file existence
- Read file with error handling
- Parse JSON with error handling
- Validate structure progressively
Each error path returns a descriptive message that will help users fix their configuration.
apps/desktop/src/main/lib/static-ports/watcher.ts (3)
38-56: Well-designed multi-level watch strategy.The three-tier approach handles all scenarios:
- Watch
ports.jsondirectly if it exists- Watch
.supersetdir for file creation- Watch worktree root for
.supersetcreationThe recursive call to
watch()when directory structure changes (lines 73, 110) elegantly promotes the watcher to the most specific target.
76-95: Good use of mtime comparison to filter spurious events.The
fs.watchAPI is notoriously noisy on some platforms. ComparingmtimeMsvalues prevents false change events triggered by access time updates when reading the file. This is a robust approach.
129-143: Comprehensive cleanup inunwatch().Properly closes the watcher, clears debounce timers, and removes stored mtime. This prevents resource leaks and stale state.
apps/desktop/src/lib/trpc/routers/ports/ports.ts (2)
47-68: Clean implementation ofhasStaticConfigprocedure.The procedure correctly:
- Validates input with zod
- Handles missing workspace gracefully
- Uses
getWorkspacePathutility for path resolution- Returns a typed response object
111-147: Correct use of observable pattern for subscription.Per coding guidelines for tRPC subscriptions with trpc-electron, the observable pattern from
@trpc/server/observableis required. The implementation correctly:
- Returns an observable that emits change events
- Sets up the file watcher on subscription start
- Properly cleans up listener and watcher on unsubscription
apps/desktop/plans/20260108-2251-static-ports-json.md (1)
1-404: Good planning documentation overall.The plan effectively documents:
- Feature scope and user impact
- Design decisions with rationale
- Implementation milestones
- Validation criteria
This will be valuable for future maintainers understanding why certain choices were made.
Adds JSDoc documentation to the validatePortEntry helper function to satisfy the docstring coverage threshold. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Update toast import path from `sonner` to `@superset/ui/sonner` - Mark Milestone 4 (Zustand store updates) as skipped with note referencing the Decision Log entry 🤖 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: 0
🧹 Nitpick comments (3)
apps/desktop/src/main/lib/static-ports/loader.ts (2)
76-149: Consider validating for duplicate port numbers.The validation doesn't check if the same port number appears multiple times in the array. While not necessarily invalid, duplicate ports could indicate a configuration error and might confuse users.
💡 Optional enhancement to detect duplicate ports
After line 145, before pushing to validatedPorts:
for (let i = 0; i < parsed.ports.length; i++) { const entry = parsed.ports[i] as PortEntry; const result = validatePortEntry(entry, i); if (!result.valid) { return { exists: true, ports: null, error: result.error }; } + // Check for duplicate port numbers + if (validatedPorts.some((p) => p.port === result.port)) { + return { + exists: true, + ports: null, + error: `Duplicate port number ${result.port} at ports[${i}]`, + }; + } + validatedPorts.push({ port: result.port, label: result.label }); }
157-164: Extract path construction to eliminate duplication.The path construction logic (lines 158-162) is duplicated from
loadStaticPorts(lines 77-81). Extract this to a helper function to follow the DRY principle.♻️ Proposed refactor to extract path construction
Add a helper function at the top of the file after the type definitions:
+/** + * Get the path to the ports.json file for a worktree. + * + * @param worktreePath - Path to the workspace's worktree directory + * @returns Full path to .superset/ports.json + */ +function getPortsFilePath(worktreePath: string): string { + return join(worktreePath, PROJECT_SUPERSET_DIR_NAME, PORTS_FILE_NAME); +} + /** * Validate a single port entry from the ports.json configuration.Then update both functions to use it:
export function loadStaticPorts(worktreePath: string): StaticPortsResult { - const portsPath = join( - worktreePath, - PROJECT_SUPERSET_DIR_NAME, - PORTS_FILE_NAME, - ); + const portsPath = getPortsFilePath(worktreePath); if (!existsSync(portsPath)) {export function hasStaticPortsConfig(worktreePath: string): boolean { - const portsPath = join( - worktreePath, - PROJECT_SUPERSET_DIR_NAME, - PORTS_FILE_NAME, - ); + const portsPath = getPortsFilePath(worktreePath); return existsSync(portsPath); }apps/desktop/plans/20260108-2251-static-ports-json.md (1)
242-250: Address markdown linting issue with code block style.The static analysis tool flagged this code block. The project's markdown configuration expects indented code blocks, but this uses fenced style.
📝 Fix markdown code block style
Change from fenced to indented code block:
- ```json - { - "ports": [ - { "port": 3000, "label": "Frontend Dev Server" }, - { "port": 8080, "label": "API Server" }, - { "port": 5432, "label": "PostgreSQL" } - ] - } - ``` + { + "ports": [ + { "port": 3000, "label": "Frontend Dev Server" }, + { "port": 8080, "label": "API Server" }, + { "port": 5432, "label": "PostgreSQL" } + ] + }Or update the markdownlint configuration to allow fenced code blocks if that's the preferred style.
Based on static analysis hints.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/plans/20260108-2251-static-ports-json.mdapps/desktop/src/main/lib/static-ports/loader.ts
🧰 Additional context used
📓 Path-based instructions (4)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/main/lib/static-ports/loader.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use object parameters for functions with 2+ parameters instead of positional arguments
Functions with 2+ parameters should accept a single params object with named properties for self-documentation and extensibility
Use prefixed console logging with context pattern: [domain/operation] message
Extract magic numbers and hardcoded values to named constants at module top
Use lookup objects/maps instead of repeated if (type === ...) conditionals
Avoid usinganytype - maintain type safety in TypeScript code
Never swallow errors silently - at minimum log them with context
Import from concrete files directly when possible - avoid barrel file abuse that creates circular dependencies
Avoid deep nesting (4+ levels) - use early returns, extract functions, and invert conditions
Use named properties in options objects instead of boolean parameters to avoid boolean blindness
Files:
apps/desktop/src/main/lib/static-ports/loader.ts
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations - never use raw SQL
Files:
apps/desktop/src/main/lib/static-ports/loader.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for formatting and linting - run at root level with
bun run lint:fixorbiome check --write
Files:
apps/desktop/src/main/lib/static-ports/loader.ts
🧬 Code graph analysis (1)
apps/desktop/src/main/lib/static-ports/loader.ts (2)
apps/desktop/src/shared/types/ports.ts (1)
StaticPortsResult(17-21)apps/desktop/src/shared/constants.ts (2)
PROJECT_SUPERSET_DIR_NAME(33-33)PORTS_FILE_NAME(36-36)
🪛 markdownlint-cli2 (0.18.1)
apps/desktop/plans/20260108-2251-static-ports-json.md
242-242: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
🔇 Additional comments (2)
apps/desktop/src/main/lib/static-ports/loader.ts (2)
1-13: LGTM: Clean imports and type-safe validation approach.The use of
unknowntypes for pre-validation interfaces (PortEntry,PortsConfig) is a type-safe pattern that allows proper narrowing during validation, avoiding the pitfalls ofany.
15-68: LGTM: Comprehensive validation logic.The validation correctly enforces the port schema: integer ports within the valid TCP range (1-65535), non-empty string labels, and provides clear error messages with array indices for debugging. Trimming the label is a nice touch.
|
I have a few improvements just checking w/ your requirements before i merge @steven-terrana.
What are you thoughts? |
Kitenite
left a comment
Kitenite
left a comment
There was a problem hiding this comment.
Approve with some follow up comments above @steven-terrana. Overall nice works, it works very well :)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/components/MergedPortBadge/MergedPortBadge.tsx:
- Around line 83-93: When rendering the active-port block in MergedPortBadge,
guard against null/undefined values for port.pid and port.processName so you
don't render "null (pid null)"; update the JSX that checks port.isActive to
substitute safe fallbacks (e.g., empty string, "unknown" or "—") for
port.processName and port.pid and only render the pid segment if pid is present,
and keep the canJumpToTerminal text unchanged; locate the conditional rendering
inside the MergedPortBadge component where port.isActive is used and replace
direct references to port.processName and port.pid with these fallback-aware
expressions.
🧹 Nitpick comments (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/PortsList.tsx (2)
178-218: ExtractMergedWorkspacePortGroupto its own file.Per coding guidelines, there should be one component per file.
MergedWorkspacePortGroupshould be moved to its own file undercomponents/.Suggested structure
Create a new file at
components/MergedWorkspacePortGroup/MergedWorkspacePortGroup.tsx:import { trpc } from "renderer/lib/trpc"; import type { MergedPort } from "shared/types"; import { MergedPortBadge } from "../MergedPortBadge"; export interface MergedWorkspaceGroup { workspaceId: string; workspaceName: string; isCurrentWorkspace: boolean; ports: MergedPort[]; } interface MergedWorkspacePortGroupProps { group: MergedWorkspaceGroup; } export function MergedWorkspacePortGroup({ group }: MergedWorkspacePortGroupProps) { // ... current implementation }Then export via
components/MergedWorkspacePortGroup/index.tsand import inPortsList.tsx.
71-87: Error deduplication ref accumulates indefinitely.The
shownErrorsRefSet is never cleared, so if a user fixes theirports.jsonand then breaks it again with the same error, the toast won't appear. Consider clearing the Set when errors are resolved, or using a time-based expiry.Possible approach
+ // Clear shown errors when the error list changes to allow re-showing + const previousErrorKeys = useRef<Set<string>>(new Set()); + useEffect(() => { const errors = allStaticPortsData?.errors ?? []; + const currentErrorKeys = new Set( + errors.map(({ workspaceId, error }) => `${workspaceId}:${error}`) + ); + + // Clear keys for errors that are no longer present + for (const key of shownErrorsRef.current) { + if (!currentErrorKeys.has(key)) { + shownErrorsRef.current.delete(key); + } + } + for (const { workspaceId, error } of errors) { const errorKey = `${workspaceId}:${error}`; if (!shownErrorsRef.current.has(errorKey)) { shownErrorsRef.current.add(errorKey); const workspaceName = workspaceNames[workspaceId] || "Unknown workspace"; toast.error(`Failed to load ports.json in ${workspaceName}`, { description: error, }); } } }, [allStaticPortsData?.errors, workspaceNames]);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/desktop/src/lib/trpc/routers/ports/ports.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/PortsList.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/components/MergedPortBadge/MergedPortBadge.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/components/MergedPortBadge/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/utils/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/utils/merge-ports.tsapps/desktop/src/shared/types/ports.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/shared/types/ports.ts
🧰 Additional context used
📓 Path-based instructions (6)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/components/MergedPortBadge/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/utils/merge-ports.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/components/MergedPortBadge/MergedPortBadge.tsxapps/desktop/src/lib/trpc/routers/ports/ports.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/utils/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/PortsList.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use object parameters for functions with 2+ parameters instead of positional arguments
Functions with 2+ parameters should accept a single params object with named properties for self-documentation and extensibility
Use prefixed console logging with context pattern: [domain/operation] message
Extract magic numbers and hardcoded values to named constants at module top
Use lookup objects/maps instead of repeated if (type === ...) conditionals
Avoid usinganytype - maintain type safety in TypeScript code
Never swallow errors silently - at minimum log them with context
Import from concrete files directly when possible - avoid barrel file abuse that creates circular dependencies
Avoid deep nesting (4+ levels) - use early returns, extract functions, and invert conditions
Use named properties in options objects instead of boolean parameters to avoid boolean blindness
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/components/MergedPortBadge/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/utils/merge-ports.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/components/MergedPortBadge/MergedPortBadge.tsxapps/desktop/src/lib/trpc/routers/ports/ports.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/utils/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/PortsList.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/components/MergedPortBadge/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/utils/merge-ports.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/components/MergedPortBadge/MergedPortBadge.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/utils/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/PortsList.tsx
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations - never use raw SQL
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/components/MergedPortBadge/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/utils/merge-ports.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/components/MergedPortBadge/MergedPortBadge.tsxapps/desktop/src/lib/trpc/routers/ports/ports.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/utils/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/PortsList.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for formatting and linting - run at root level with
bun run lint:fixorbiome check --write
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/components/MergedPortBadge/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/utils/merge-ports.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/components/MergedPortBadge/MergedPortBadge.tsxapps/desktop/src/lib/trpc/routers/ports/ports.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/utils/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/PortsList.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
One component per file - do not create multi-component files
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/components/MergedPortBadge/MergedPortBadge.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/PortsList.tsx
🧠 Learnings (3)
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/desktop/src/renderer/**/*.{ts,tsx} : Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/utils/index.ts
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/desktop/src/lib/*.ts : Never import Node.js modules in shared code like electron-router-dom.ts - it runs in both main and renderer processes
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/utils/index.ts
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/desktop/src/lib/electron-router-dom.ts : Do not import Node.js modules like node:path or dotenv in electron-router-dom.ts and similar shared files - they run in both main and renderer processes
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/utils/index.ts
🧬 Code graph analysis (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/utils/merge-ports.ts (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/utils/index.ts (1)
mergePorts(1-1)apps/desktop/src/shared/types/ports.ts (3)
StaticPort(11-15)DetectedPort(1-9)MergedPort(23-33)
apps/desktop/src/lib/trpc/routers/ports/ports.ts (7)
apps/desktop/src/main/lib/local-db/index.ts (1)
localDb(82-82)packages/local-db/src/schema/schema.ts (1)
workspaces(80-118)apps/desktop/src/lib/trpc/routers/workspaces/utils/worktree.ts (1)
getWorkspacePath(22-43)apps/desktop/src/main/lib/static-ports/index.ts (3)
hasStaticPortsConfig(1-1)loadStaticPorts(1-1)staticPortsWatcher(2-2)apps/desktop/src/main/lib/static-ports/loader.ts (2)
hasStaticPortsConfig(157-164)loadStaticPorts(76-149)apps/desktop/src/shared/types/ports.ts (1)
StaticPort(11-15)apps/desktop/src/main/lib/static-ports/watcher.ts (1)
staticPortsWatcher(155-155)
🔇 Additional comments (8)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/utils/index.ts (1)
1-1: LGTM!Clean barrel export for the utils module public API.
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/components/MergedPortBadge/index.ts (1)
1-1: LGTM!Standard component barrel export.
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/utils/merge-ports.ts (1)
12-61: LGTM!Well-structured merge logic with clear documentation. The function correctly:
- Builds efficient lookup structures (Map/Set) for O(1) access
- Merges static port definitions with dynamic runtime data
- Preserves static ports even when inactive
- Adds dynamic-only ports that aren't in static config
- Sorts the final result by port number
The caller is responsible for ensuring
staticPortsare pre-filtered for the target workspace, which is handled correctly inPortsList.tsx(line 107-109).apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/components/MergedPortBadge/MergedPortBadge.tsx (1)
13-41: LGTM!Clean component implementation with proper:
- Async handling with appropriate guards
- Store access patterns using zustand
- tRPC mutation usage for workspace switching
- Focus and accessibility considerations
apps/desktop/src/lib/trpc/routers/ports/ports.ts (2)
141-175: LGTM - well-structured subscription with proper cleanup.The observable pattern is correctly used per coding guidelines. The cleanup function properly unsubscribes from the watcher and removes the listener.
One minor note: the early returns on lines 151-153 and 156-158 silently do nothing for invalid workspaces. Consider whether a debug log would help with troubleshooting subscription issues.
106-139: LGTM - good error aggregation pattern.The bulk query correctly aggregates ports from all workspaces while collecting per-workspace errors separately. This allows the UI to display available ports even when some workspaces have invalid configurations.
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/PortsList.tsx (2)
32-40: Static port subscription is scoped to active workspace only.The
subscribeStaticsubscription only watches the active workspace'sports.json. Changes to other workspaces' configs won't trigger a refresh until the user switches to them. This aligns with the current implementation but may be relevant to the PR discussion about expanding fetch scope to all workspaces.If all-workspace live updates are desired in the future, consider subscribing to each workspace or implementing a global watcher that emits changes for any workspace.
103-143: LGTM - efficient merged port computation.Good use of
useMemowith correct dependency array. The merge logic properly:
- Filters static ports per workspace before merging
- Sorts groups with current workspace first
- Computes total count from merged results
My use case largely centers around a so for me, the |
That would still be compatible, but it would just show for every existing ports.json. Perhaps it'll be a little verbose 
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/PortsList.tsx (2)
206-241: ExtractMergedWorkspacePortGroupto its own file.This file contains two components (
PortsListandMergedWorkspacePortGroup), which violates the coding guideline requiring one component per file.As per coding guidelines:
One component per file - do not create multi-component files
📁 Recommended file structure
Create a new file:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/components/MergedWorkspacePortGroup/MergedWorkspacePortGroup.tsxMove lines 202-241 to the new file, and add the corresponding barrel export:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/components/MergedWorkspacePortGroup/index.tsThen import it in PortsList.tsx:
import { MergedWorkspacePortGroup } from "./components/MergedWorkspacePortGroup";
210-215: Add error handling for workspace switching.Similar to the issue in
MergedPortBadge, this async mutation lacks error handling. If the workspace switch fails, the user won't receive feedback.🛡️ Proposed fix with error handling
const handleWorkspaceClick = async () => { if (group.isCurrentWorkspace) return; + try { await setActiveMutation.mutateAsync({ id: group.workspaceId }); await utils.workspaces.getActive.invalidate(); + } catch (error) { + console.error('[PortsList/handleWorkspaceClick] Failed to switch workspace:', error); + toast.error('Failed to switch workspace', { + description: 'Could not activate the selected workspace' + }); + } };Note:
toastis already imported at line 2.
🤖 Fix all issues with AI agents
In
@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/components/MergedPortBadge/MergedPortBadge.tsx:
- Around line 39-52: Wrap the workspace switch logic in handleClick with a
try/catch around setActiveMutation.mutateAsync and the subsequent invalidate: if
mutateAsync throws, catch the error, show a user-facing toast (import toast from
@superset/ui/sonner) with a helpful message and return early. After a successful
mutation+invalidate, re-read panes from useTabsStore.getState() (do not reuse a
stale reference) before resolving pane with port.paneId; only call setActiveTab
and setFocusedPane when pane is present. Ensure errors in any of these steps are
caught and reported via toast.
In
@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/PortsList.tsx:
- Around line 75-91: The deduplication set shownErrorsRef never gets cleared so
recurring errors are suppressed; update the useEffect that processes
allStaticPortsData?.errors to reconcile shownErrorsRef with the current errors:
build a Set of current error keys (using `${workspaceId}:${error}`), remove any
keys from shownErrorsRef.current that are no longer present (or call
shownErrorsRef.current.clear() when there are no errors), then add and toast
only new keys as currently done; keep this logic inside the same useEffect that
references allStaticPortsData?.errors and workspaceNames so shownErrorsRef is
kept in sync with resolved/recurring errors.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/components/MergedPortBadge/MergedPortBadge.tsx (1)
39-52: Consider early returns to reduce nesting.The function has multiple conditional checks that could be flattened using early returns for improved readability.
♻️ Alternative structure with early returns
const handleClick = async () => { if (!canJumpToTerminal || !port.paneId) return; try { if (!isCurrentWorkspace) { await setActiveMutation.mutateAsync({ id: port.workspaceId }); await utils.workspaces.getActive.invalidate(); } const pane = useTabsStore.getState().panes[port.paneId]; - if (!pane) return; - - setActiveTab(port.workspaceId, pane.tabId); - setFocusedPane(pane.tabId, port.paneId); + if (!pane) { + console.warn('[MergedPortBadge/handleClick] Pane not found:', port.paneId); + return; + } + + setActiveTab(port.workspaceId, pane.tabId); + setFocusedPane(pane.tabId, port.paneId); } catch (error) { console.error('[MergedPortBadge/handleClick] Failed to switch workspace:', error); toast.error('Failed to open workspace', { description: 'Could not switch to the target workspace' }); } };
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/PortsList.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/components/MergedPortBadge/MergedPortBadge.tsx
🧰 Additional context used
📓 Path-based instructions (6)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/components/MergedPortBadge/MergedPortBadge.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/PortsList.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use object parameters for functions with 2+ parameters instead of positional arguments
Functions with 2+ parameters should accept a single params object with named properties for self-documentation and extensibility
Use prefixed console logging with context pattern: [domain/operation] message
Extract magic numbers and hardcoded values to named constants at module top
Use lookup objects/maps instead of repeated if (type === ...) conditionals
Avoid usinganytype - maintain type safety in TypeScript code
Never swallow errors silently - at minimum log them with context
Import from concrete files directly when possible - avoid barrel file abuse that creates circular dependencies
Avoid deep nesting (4+ levels) - use early returns, extract functions, and invert conditions
Use named properties in options objects instead of boolean parameters to avoid boolean blindness
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/components/MergedPortBadge/MergedPortBadge.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/PortsList.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/components/MergedPortBadge/MergedPortBadge.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/PortsList.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
One component per file - do not create multi-component files
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/components/MergedPortBadge/MergedPortBadge.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/PortsList.tsx
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations - never use raw SQL
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/components/MergedPortBadge/MergedPortBadge.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/PortsList.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for formatting and linting - run at root level with
bun run lint:fixorbiome check --write
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/components/MergedPortBadge/MergedPortBadge.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/PortsList.tsx
🧬 Code graph analysis (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/components/MergedPortBadge/MergedPortBadge.tsx (3)
apps/desktop/src/shared/types/ports.ts (1)
MergedPort(23-33)packages/ui/src/components/ui/tooltip.tsx (3)
Tooltip(76-76)TooltipTrigger(76-76)TooltipContent(76-76)apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/constants.ts (1)
STROKE_WIDTH(2-2)
🔇 Additional comments (4)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/components/MergedPortBadge/MergedPortBadge.tsx (1)
1-112: Overall structure looks good.The component properly:
- Uses React 19 patterns (no forwardRef needed for ref prop)
- Leverages Tooltip from @superset/ui correctly
- Implements conditional styling and accessibility attributes
- Uses null-safe checks (
port.pid != nullat line 96)apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/PortsList.tsx (3)
93-142: Excellent use of memoization for complex derived state.The memoization strategy for
allWorkspaceIdsandworkspacePortGroupsis well-designed:
- Dependencies are correctly specified
- Expensive operations (filtering, merging, sorting) are cached
- The merge logic properly combines static and dynamic ports per workspace
The sorting logic (current workspace first, then alphabetical) provides a good UX.
36-44: Good use of tRPC subscription for live reload.The static ports subscription with query invalidation provides a clean reactive pattern for file system changes without manual polling.
153-189: Well-implemented help tooltip with docs link.The help icon with tooltip provides good discoverability for the static port configuration feature. The reveal-on-hover pattern (
opacity-0 group-hover:opacity-100) is a nice touch.
|
@steven-terrana Merged with some of the improvements I talked about. Let me know if there's any of the changes you don't like when its published. Thanks for the PR! |
feat(desktop): add static port configuration via .superset/ports.json Allow users to define static ports in a .superset/ports.json file within their workspace, replacing dynamic port discovery when present. Add static-ports module with loader and file watcher Add tRPC procedures: hasStaticConfig, getStatic, subscribeStatic Auto-reload config on file changes with error toast notifications Add marketing documentation page at /ports
2 participants
Links
apps/desktop/plans/20260108-2251-static-ports-json.mdSummary
.superset/ports.jsonto replace dynamic port discovery/portsexplaining the featureWhy / Context
Superset automatically detects ports from terminal processes, but some users need:
Static ports allow users to define their own port list with descriptive labels that the whole team can use.
How It Works
.superset/ports.jsonin the worktreeKey files:
static-ports/loader.ts- JSON parsing and validationstatic-ports/watcher.ts- fs.watch with debouncing for live reloadports.ts(router) - tRPC procedures for hasStaticConfig, getStatic, subscribeStaticPortsList.tsx- Conditional rendering based on static vs dynamic modeManual QA Checklist
Static Config Detection
.superset/ports.jsonswitches to static mode (may need to refocus workspace)Static Ports Display
localhost:PORTin browserLive Reload
Error Handling
portskey shows toast: "ports.json is missing required field 'ports'"Marketing Page
Testing
bun run typecheck✅bun run lint✅bun test apps/desktop/src/main/lib/static-ports/loader.test.ts- 24 tests passingDesign Decisions
Known Limitations
Follow-ups
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.