feat(desktop): add ability to close ports from ports list#797
Conversation
Add close buttons to the ports sidebar section allowing users to kill processes listening on specific ports directly from the UI.
|
Warning Rate limit exceeded
⌛ 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 (5)
📝 WalkthroughWalkthroughThis PR adds process termination capability to the ports management system. It introduces a new TRPC endpoint that kills processes by PID, a React hook managing port termination with error handling, and UI buttons in the sidebar to close individual ports or entire workspace port groups. Changes
Sequence DiagramsequenceDiagram
actor User
participant UI as UI Component<br/>(MergedPortBadge/<br/>WorkspacePortGroup)
participant Hook as useKillPort<br/>Hook
participant TRPC as TRPC Client
participant Server as Backend<br/>ports.ts
participant Process as Node<br/>Process
participant Toast as Toast<br/>Notification
User->>UI: Click close port(s) button
UI->>Hook: killPort(port) or killPorts(ports)
Hook->>Hook: Filter active ports with PID
Hook->>TRPC: Call electronTrpc.kill({ pid })
TRPC->>Server: Send kill mutation request
Server->>Process: process.kill(pid, 'SIGTERM')
alt Success
Process-->>Server: Process terminated
Server-->>TRPC: { success: true }
TRPC-->>Hook: Success response
Hook->>Toast: Show success (implicit)
else Failure
Process-->>Server: Error occurred
Server-->>TRPC: { success: false, error: "..." }
TRPC-->>Hook: Error response
Hook->>Toast: Show error message
end
Toast->>User: Display result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Add text-muted-foreground base color and hover states to external link (hover:text-primary) and close (hover:text-destructive) buttons.
b87c77e to
3b85a1c
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/desktop/src/lib/trpc/routers/ports/ports.ts`:
- Line 1: The PID schema currently uses z.object({ pid: z.number() }) which
allows decimals, zero and negatives and can cause dangerous process-group
signalling when used with process.kill; update the schema (in ports.ts where the
PID input is defined/used) to require a positive integer (e.g., use
z.number().int().positive() or z.number().int().min(1) / z.int().positive()) so
only valid PIDs >= 1 are accepted, and ensure any handler that calls
process.kill() uses this validated value.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/hooks/useKillPort.ts`:
- Around line 5-35: The killPort and killPorts functions in useKillPort can
throw unhandled rejections from killMutation.mutateAsync; wrap both killPort and
killPorts bodies in try/catch blocks, log errors with a “[ports/kill]” prefix
using console.error (or the project logger) and show user-facing toasts on
failure; in killPorts use Promise.allSettled instead of Promise.all to allow all
mutations to run, then treat settled results by filtering both rejected promises
and settled values where result.success === false, and surface the total
failures in the toast; keep the function names (useKillPort, killPort,
killPorts) and the killMutation.mutateAsync calls but add the described error
handling and logging.
- Move process.kill logic to portManager.killPort() - Accept paneId + port instead of raw PID from client - Verify PID is not the terminal shell before killing - Only kill processes tracked in our ports map
|
feat(desktop): improve port management UI with close buttons and safer process handling Add close buttons to ports sidebar with improved interaction patterns Move process.kill logic to dedicated portManager.killPort() method |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Summary
useKillPorthookTest plan
bun dev)Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.