Skip to content

feat(ssh): show saved port forwards that couldn't start, with why, and let you remove them - #33450

Merged
nwparker merged 9 commits into
mainfrom
nwparker/fix-32484-unrestored-forwards-list
Oct 25, 2026
Merged

nwparker merged 9 commits into
mainfrom
nwparker/fix-32484-unrestored-forwards-list

Conversation

@nwparker

@nwparker nwparker commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 15 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​238 $\color{#cf222e}{\Huge{\mathbf{−}}}$​5 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​233
Prod 33 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​490 $\color{#cf222e}{\Huge{\mathbf{−}}}$​50 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​440

ELI5

Some saved SSH port forwards can't start: their local port is already in use, the port falls in a range Windows reserves, or the ssh process keeps exiting. Orca keeps those forwards saved and retries them in the background, but the Ports panel only listed open forwards. So you couldn't see them, and you couldn't remove them. Now they're listed with the reason they didn't start, and you can remove them.

Builds on #33388, which has merged; this PR is rebased onto main.

What Changed

  • Before: a saved forward that wasn't open was invisible. It stayed saved forever and was retried in the background, and nothing in the UI could remove it. An ssh that started and later exited was also retried every 30 s with no growing wait.
  • After:
    • Both port-forward lists (the Ports panel and the Forwarded section a managed server's workspace shows) list each saved forward that isn't open. The row shows "Couldn't start: " when the last start failed, or "Not started yet", plus a Remove button.
    • Remove deletes the forward by its mapping (local port, remote host, remote port), through the same per-host queue as every other forward change.
    • A forward whose ssh exits after starting is recorded as a failure, so it uses the same backoff as a forward that never started (30 s, then 60 s, up to 10 min).
    • A forward that restarts and dies again keeps its failure streak, so its wait keeps growing. The streak only resets once the forward has stayed up for 5 minutes.
    • Remove also closes the forward if a restore queued ahead of the click had just opened it.
    • A forward whose ssh ran and then died is labelled "Stopped: "; one that never came up says "Couldn't start: ".
    • The list stays visible, and Remove still works, while the host is offline. That includes hosts that never connect this session. The offline panel scrolls when the list is long.
    • Adding, editing or removing a forward by hand resets its backoff.
    • A failed Remove shows an error toast.
    • A start that fails on a connection already being torn down records no failure.
  • Mechanism:
    • The port-forwards-changed broadcast carries an optional unrestored list. It goes from main to this app's own renderer, so client and host versions don't matter.
    • Startup reads it for every saved host through a new ssh:listUnrestoredPortForwards call, and reads it again when a host disconnects. A read that a newer push has already replaced is dropped.
    • ssh:removeSavedPortForward removes a forward by its mapping.
    • The web client's stubs return nothing.

Why

Hiding these forwards leaves users with no way to stop a forward that can never start, and no clue why it's missing. Listing them next to the open forwards, with the last error, reuses the existing list and row styling: muted text, text-destructive for the error state, and the same remove button. Mobile has no port-forward list, so there's nothing to match there.

Linked Issue

Refs #32484 (follow-up to #33388)

Visual Proof

The Docker e2e takes a screenshot of the row (unrestored-forward-row.png in the run's artifacts). Taken in a hidden window.

Testing

  • Automated tests added/updated
  • Docker e2e "a saved forward whose local port is taken after a restart shows why, and can be removed":
    • adds a forward, quits, and holds its local port from the test process;
    • relaunches, then checks that the row appears saying "Couldn't start";
    • clicks Remove, then checks that the saved forward is gone.
  • Unit tests:
    • a forward that keeps failing is listed with its error;
    • a forward that started and then exited backs off;
    • the close callback records the exit and broadcasts the forward as not open, with its error;
    • Remove by mapping goes through the queue and persists the drop.
  • Tests that import the changed modules, run single-worker: 1185 files pass. A pre-existing teardown flake in unrelated store tests shows the same error count as before this PR. The full static job runs locally against the fix(ssh): saved port forwards come back after a restart that reconnects through the host's Orca server (#32484) #33388 head.

Review

Codex review: the final head is clean against the merge-base with main. Earlier rounds found these, all fixed here: the two new channels were missing from IPC re-registration; Remove could lose to a restore that had just opened the forward; hydration fixtures needed the new call; the empty state showed next to saved rows; a flapping ssh reset its backoff; offline hosts weren't loaded and stale reads could win; the offline list couldn't scroll. An independent Opus review approved the PR (no P0/P1); its P2/P3 items are fixed here: every locale has the strings, the existing Remove key is reused, a failed remove shows a toast, a dead forward is labelled Stopped, manual edits reset the backoff, the list survives going offline, the button has an aria-label, the healthy-uptime reset is tested, and a test that only checked its own mocks is removed.

Agent skill upstream boundary

  • Not applicable

Notes

Cross-platform: no platform-specific code; the reason text comes from the OS error. SSH/remote: client-only, with optional fields. Folder workspaces: forwards belong to the SSH host, not the workspace.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • Visual proof via the e2e screenshot
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered
  • Typecheck, lint and focused tests pass locally

…d let you remove them (#32484)

A saved forward whose local port is taken, or that keeps failing, stayed
saved but invisible, retried in the background with no way to remove it.
The port-forward lists now show each saved forward that isn't open, with
the last start error, and Remove deletes it by its mapping.
…tore reopened before removing it, and update IPC fixtures
…that died, toast a failed remove, and reset backoff on manual edits
…offline; label the remove button; test the healthy-uptime reset
@nwparker
nwparker force-pushed the nwparker/fix-32484-unrestored-forwards-list branch from 497ace9 to fb54122 Compare October 25, 2026 09:18
@nwparker
nwparker requested a review from Jinwoo-H as a code owner October 25, 2026 09:18
@nwparker
nwparker changed the base branch from nwparker/fix-32484-forward-persistence to main October 25, 2026 09:18
@coderabbitai

coderabbitai Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 93624826-e835-4cff-bf5e-8aedd759393e
Commits

Reviewing files that changed from the base of the PR and between fb54122 and d29945f.

Files selected for processing: 1
  • tests/e2e/ssh-orcad-workspace-tools.spec.ts
Files skipped from review as they are similar to previous changes: 1
  • tests/e2e/ssh-orcad-workspace-tools.spec.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

Walkthrough

SSH port-forward failures now retain error and retry details, and the IPC API can list or remove saved forwards that are not open. Renderer state receives these forwards through hydration and change events. The Ports UI displays their endpoints and status and provides a removal action. Tests cover retry timing, synchronization, handler behavior, and a failed restore across an app restart.

Priority: Normal

Merge Risk: Minimal · up to d2994

No actionable merge-blocking risk is identified; the change is ready for normal merge checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 38.71% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 31 functions across 41 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: showing saved SSH port forwards that failed to start and allowing users to remove them.
Description check Passed The description is complete and aligned with the template. It explains the user impact, implementation, rationale, linked issue, visual proof, testing, compatibility considerations, and review status.…
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.
Full details: Description check

Explanation

The description is complete and aligned with the template. It explains the user impact, implementation, rationale, linked issue, visual proof, testing, compatibility considerations, and review status. Some checklist items, such as manual testing and build verification, are not explicitly confirmed, but the description is otherwise sufficiently detailed.

Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


Review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e29ccf9f-78b5-4751-9798-4121f86b6070
Commits

Reviewing files that changed from the base of the PR and between 4dcd6af and fb54122.

Files selected for processing: 48
  • src/main/ipc/ssh/ssh-handler-reregistration.test.ts
  • src/main/ipc/ssh/ssh-port-forward-handlers.test.ts
  • src/main/ipc/ssh/ssh-port-forward-handlers.ts
  • src/main/ipc/ssh/ssh-port-forward-persistence.ts
  • src/main/ipc/ssh/ssh-port-forward-restore.test.ts
  • src/main/ipc/ssh/ssh-port-forward-unrestored.ts
  • src/main/ipc/ssh/ssh-renderer-broadcast.ts
  • src/main/ipc/ssh/ssh.ts
  • src/preload/api/ssh-api.ts
  • src/preload/api/ssh-bridge.ts
  • src/renderer/src/components/right-sidebar/ssh-forwarded-ports-section.tsx
  • src/renderer/src/components/right-sidebar/ssh-ports-panel.tsx
  • src/renderer/src/components/right-sidebar/ssh-unrestored-port-row.tsx
  • src/renderer/src/hooks/ipc-events-agent-status-store-test-fixtures.ts
  • src/renderer/src/hooks/ipc-events-agent-status-window-test-fixtures.ts
  • src/renderer/src/hooks/ipc-events-close-routing-test-harness.ts
  • src/renderer/src/hooks/ipc-events-harness-store-state.ts
  • src/renderer/src/hooks/ipc-events-ssh-api-test-fixtures.ts
  • src/renderer/src/hooks/ipc-events-terminal-create-scenario-types.ts
  • src/renderer/src/hooks/ipc-events-terminal-create-test-harness.ts
  • src/renderer/src/hooks/ipc-events-terminal-create-window-test-fixtures.ts
  • src/renderer/src/hooks/ipc-events-test-harness.ts
  • src/renderer/src/hooks/ipc-events/direct-ssh-connect-reply-routing.test.ts
  • src/renderer/src/hooks/ipc-events/direct-ssh-hydration-fanout.test.ts
  • src/renderer/src/hooks/ipc-events/direct-ssh-hydration-target-metadata.test.ts
  • src/renderer/src/hooks/ipc-events/direct-ssh-initial-state-hydration.ts
  • src/renderer/src/hooks/ipc-events/direct-ssh-state-ipc-bridge.ts
  • src/renderer/src/hooks/ipc-events/direct-ssh-unrestored-forward-sync.test.ts
  • src/renderer/src/hooks/ipc-events/direct-ssh-unrestored-forward-sync.ts
  • src/renderer/src/hooks/useIpcEvents-agent-status-ssh-authority.test.ts
  • src/renderer/src/hooks/useIpcEvents-browser-tab-create.test.ts
  • src/renderer/src/hooks/useIpcEvents-cli-worktree-activation.test.ts
  • src/renderer/src/hooks/useIpcEvents-close-routing-browser-pages.test.ts
  • src/renderer/src/hooks/useIpcEvents-rate-limit-hydration.test.ts
  • src/renderer/src/hooks/useIpcEvents-ssh-disconnect-cleanup.test.ts
  • src/renderer/src/hooks/useIpcEvents-updater-status.test.ts
  • src/renderer/src/i18n/locales/en.json
  • src/renderer/src/i18n/locales/es.json
  • src/renderer/src/i18n/locales/fr.json
  • src/renderer/src/i18n/locales/ja.json
  • src/renderer/src/i18n/locales/ko.json
  • src/renderer/src/i18n/locales/pt.json
  • src/renderer/src/i18n/locales/zh.json
  • src/renderer/src/store/slices/ssh-target-cleanup.ts
  • src/renderer/src/store/slices/ssh.ts
  • src/renderer/src/web/preload-api/web-terminal-api.ts
  • src/shared/ssh-types.ts
  • tests/e2e/ssh-orcad-workspace-tools.spec.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

@nwparker
nwparker merged commit 0b28553 into main Oct 25, 2026
49 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant