Skip to content

fix(desktop): confirm before closing active ports - #6007

Merged
saddlepaddle merged 1 commit into
ParadevAI:mainfrom
Roshvan:kill-port-warning
Jul 28, 2026
Merged

saddlepaddle merged 1 commit into
ParadevAI:mainfrom
Roshvan:kill-port-warning

Conversation

@Roshvan

@Roshvan Roshvan commented Jul 27, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • add the existing destructive alert pattern before closing active ports
  • guard both individual port close actions and “Close all ports” through the shared port-kill hook
  • share the running-process “Don’t ask again” preference with terminal-close confirmations
  • cover singular, plural, cancel, dismiss, suppression, and unavailable-alert behavior

Why

The ports hover card called the kill path immediately. “Close all ports” could therefore terminate running processes with no confirmation, even though terminal and agent close flows already protect the same destructive outcome.

The root cause was that usePortKillActions delegated directly to killPortTarget without a destructive-action guard. Centralizing the guard in that hook covers both sidebar implementations and prevents individual port buttons from bypassing it.

Impact

Users now see a destructive confirmation before closing one or many active ports. Cancel and dismiss preserve the processes; confirming keeps the existing kill and refresh behavior. The optional suppression setting stays consistent with other running-process warnings.

Verification

  • bun run lint:fix
  • bun run lint
  • bun test apps/desktop/src/renderer/hooks/ports/usePortKillActions/confirmClosePorts.test.ts apps/desktop/src/renderer/hooks/ports/usePortKillActions/killPortTarget.test.ts
  • bun run --cwd apps/desktop typecheck
  • exact-worktree Electron/CDP journey on renderer http://localhost:3105, workspace route /v2-workspace/06367ee2-0f02-4525-a2b8-f4a23923c0ea
    • baseline: clicking “Close all ports” terminated listeners on 58278 and 58279 without a confirmation
    • fixed cancel path: both listeners remained active
    • fixed confirm path: both listeners stopped and the active-port chip disappeared
    • individual port action displayed the singular warning and preserved both listeners on cancel

Screenshots are attached in the visual verification comment below.


Open in Stage

Summary by cubic

Add a confirmation before closing active ports in the desktop app. Protects both single-port and “Close all ports” actions, matching terminal close behavior and preventing accidental process kills.

  • Bug Fixes
    • Added a destructive confirm step in the shared port-kill hook.
    • Reused the terminal “Don’t ask again” setting for port closes.
    • Supported singular/plural copy, cancel, dismiss, and unavailable-alert fallback.
    • Added unit tests for confirm, cancel, dismiss, suppression, and fallback paths.

Written for commit ec2814c. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features
    • Added confirmation prompts before closing processes associated with ports.
    • Supports single- and multi-port messaging, cancellation, and a “Don’t ask again” option.
    • Closing actions are skipped when confirmation is canceled, while existing failure notifications remain available.

@coderabbitai

coderabbitai Bot commented Jul 27, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Port termination now requires confirmation through a shared, suppressible alert flow. Single and bulk kill actions stop on cancellation, while tests cover alert copy, dismissal, suppression persistence, and unavailable-alert fallback.

Changes

Port termination confirmation

Layer / File(s) Summary
Confirmation helper and coverage
apps/desktop/src/renderer/hooks/ports/usePortKillActions/confirmClosePorts.ts, apps/desktop/src/renderer/hooks/ports/usePortKillActions/confirmClosePorts.test.ts
Adds confirmClosePorts with contextual alert text, suppression persistence, cancellation handling, and tests for confirmation outcomes and fallback behavior.
Kill action integration
apps/desktop/src/renderer/hooks/ports/usePortKillActions/usePortKillActions.ts
Gates single and bulk port termination behind confirmation and returns cancellation results without attempting kills.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant usePortKillActions
  participant confirmClosePorts
  participant Alert
  participant PortKilling
  Caller->>usePortKillActions: invoke port kill action
  usePortKillActions->>confirmClosePorts: request confirmation
  confirmClosePorts->>Alert: display confirmation
  Alert-->>confirmClosePorts: confirm or cancel
  confirmClosePorts-->>usePortKillActions: return boolean
  alt confirmed
    usePortKillActions->>PortKilling: terminate selected ports
    PortKilling-->>Caller: return kill result
  else cancelled
    usePortKillActions-->>Caller: return undefined or empty array
  end
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title is concise, conventional, and accurately reflects the main change: adding confirmation before closing active ports.
Description check ✅ Passed The description covers what changed, why, impact, and verification, so it mostly satisfies the template despite an incomplete checklist.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@ghost

ghost commented Jul 27, 2026

Copy link
Copy Markdown

Ready to review this PR? Stage has broken it down into 2 individual chapters for you:

Title
1 Implement port closure confirmation logic
2 Integrate confirmation guard into port kill hook
Open in Stage

Chapters generated by Stage for commit ec2814c on Jul 27, 2026 11:57pm UTC.

@Roshvan
Roshvan marked this pull request as ready for review July 28, 2026 00:02
@Roshvan

Roshvan commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Visual verification

Baseline close-all affordance, bulk confirmation, and single-port confirmation:

port-warning-before-popover port-warning-after-confirmation port-warning-after-single-confirmation

@Roshvan
Roshvan marked this pull request as draft July 28, 2026 00:05
@saddlepaddle
saddlepaddle marked this pull request as ready for review July 28, 2026 00:10
@saddlepaddle
saddlepaddle merged commit 36aa907 into ParadevAI:main Jul 28, 2026
11 checks passed
@Roshvan
Roshvan deleted the kill-port-warning branch October 11, 2026 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants