Skip to content

fix(ssh): the SSH password dialog no longer closes itself and comes back - #28883

Merged
OrcaWin merged 2 commits into
mainfrom
OrcaWin/ssh-password-dismiss
Oct 15, 2026
Merged

OrcaWin merged 2 commits into
mainfrom
OrcaWin/ssh-password-dismiss

Conversation

@OrcaWin

@OrcaWin OrcaWin commented Oct 14, 2026 •

Copy link
Copy Markdown
Collaborator

ELI5

On a password host, the SSH password dialog could close by itself while the user was still about to type, then open again right away. That could keep repeating. Now the dialog stays open until the user answers or cancels. One answer or one Cancel settles it.

What Changed

Before: a user left the password dialog open for a while, or the app retried in the background. The dialog vanished and a new one appeared. This is the "dismiss + reappears" loop a user reported again on 1.4.225. #28402 already handled a Cancel. It did not cover a dialog that closed itself, because a self-closed dialog never counts as a Cancel.

After: the open dialog is the one the user answers. What the user types signs in. A Cancel stops all automatic sign-in attempts until the user clicks Reconnect.

Mechanism. The fix covers four ways one dialog could be replaced by another:

  1. The host gives up on a slow login (ssh-connection.ts). sshd drops a login nobody answers within its grace time. That is 120 s by default and often 30 to 60 s. The dropped socket counted as a network error, so the connection retried. The retry closed the open dialog and opened a new one. Now, if the startup fails while a password prompt is open, the attempt waits for the user's answer:
    • A typed password is kept for the retry, which then signs in without asking again.
    • A Cancel ends the attempt as cancelled, so it is not retried.
      A new wire test against a real ssh2 server reproduces this. It failed before the change (2 prompts) and passes after it (1 prompt).
  2. A background dial replaces a reconnect that is asking the user (ssh-connection-manager.ts). Status checks and tunnels call connect(). When the existing connection was in its own reconnect, connect() tore it down, which closed its dialog, and dialed again, which opened another. Now a connection counts as "already in progress", the same as a first connect, in two cases:
    • it has a dialog open;
    • its retry still holds a password the user just typed and the host has not accepted yet.
      The dialog is modal, so any connect that arrives while it is open is a background one.
  3. More than one prompt queued for the same host (ssh-passphrase.ts). Before, a Cancel answered only the front prompt, and the next one for the same host opened straight away. Now a Cancel closes every open prompt for that host.
  4. Paired and web clients (ssh.connect RPC). The RPC dropped the automatic flag. A web client's startup restore therefore counted as a user Reconnect, which lifted the host's Cancel. automatic is now an optional RPC field, and the web client passes it through. Old servers ignore the field. Old clients send nothing, which keeps today's "explicit" behavior.

Why

  • Waiting for the answer, over keeping the dialog open across retries. Keeping it open would mean moving the prompt out of the attempt that owns it. The attempt lifecycle and the generation fencing were built around that ownership. Waiting is local to the startup handshake. It also reuses the existing per-attempt password cache, so the retry needs no new state.
  • "Already in progress" over another automatic/explicit flag on connect(). The dialog is modal, so nobody can click Reconnect while it is open. A connection waiting on the user is a fact the connection already knows. That makes it the single check point, so any future background caller gets the same treatment.
  • Not done: one Cancel covering every saved target that points at the same host. Two saved targets for the same user@host can use different keys. Blocking both from one Cancel would break a target that signs in with a key and never prompts.

I checked the 1.4.225 hand-port of #28402 against main for every file it touched. It matches. The dropped WSL lines are unrelated to the decline.

Linked Issue

STA-10477 (follow-up)

Visual Proof

N/A. The existing dialog is reused. The change is that it no longer closes itself and reopens.

Testing

  • src/main/ssh/ssh-credential-prompt-login-grace-wire.test.ts: a real ssh2 server drops the first login while the password prompt is open. Before the change it showed 2 prompts. After it shows 1 prompt, the typed password signs in, and a Cancel ends the attempt as auth-failed.

  • src/main/ssh/ssh-connection-manager.test.ts: a background connect() leaves a connection whose reconnect is asking the user alone.

  • src/main/ipc/ssh-passphrase.test.ts: one Cancel closes the other open prompts for that host only.

  • src/main/runtime/rpc/methods/ssh.test.ts and src/renderer/src/web/web-preload-api-ssh.test.ts: automatic reaches the host's connect.

  • src/main/ssh/ssh-connection-manager-sign-in-reuse.test.ts is updated. A dial during the open prompt is now refused, and a re-asked password is still not handed on.

  • End to end, built app. The app ran hidden (ORCA_BACKGROUND_LAUNCH=1) with an isolated profile and was driven over CDP. The host was a local password-only SSH server with a 20 s login grace:

    • Before (main): the host dropped the unanswered login at 20 s. The dialog closed and a new one opened 2 s later. That was 2 prompts for one sign-in.
    • After: the dialog stayed open past the drop. The password typed 30 s in signed in on the retry without asking again. That was 1 prompt.
    • Cancel, then an automatic connect every 20 s for 3 minutes: no dialog and no new login attempts on the host.
  • Passed: pnpm tc:node, pnpm tc:web, pnpm test src/main/ssh src/main/ipc src/renderer/src/web, and pnpm run check:code-quality:changed.

  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below

AI Disclosure

Review

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

  • SSH: this is SSH-only.
  • Remote wire: automatic is a new optional field, so it is safe in both directions.
  • Platforms: no platform-specific code.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

A host that drops an unanswered login no longer replaces the open prompt
with a new one, a background dial no longer replaces a connection whose
reconnect is asking the user, one Cancel closes every open prompt for that
host, and a paired client's automatic connect stays automatic.

Co-Authored-By: Claude <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: fc937a97-19e4-46ab-9ab9-85f5ed724162
📥 Commits

Reviewing files that changed from the base of the PR and between c982f9a and 835847f.

📒 Files selected for processing: 2
  • src/main/ssh/ssh-connection.ts
  • src/main/ssh/ssh-credential-prompt-login-grace-wire.test.ts

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

📝 Walkthrough

SSH credential requests now close together when a request is declined for the same target. SSH connection startup tracks credential prompts and waits for an active keyboard-interactive answer before failing. The SSH connect RPC accepts and forwards an optional automatic flag. The connection manager rejects a repeated connection attempt while the existing connection awaits credentials.

Priority: ⬇️ Low

Merge Risk: 🔵 Low · up to 83584

A background SSH connection can replace an unfinished reconnect after a non-password prompt is answered, potentially requiring another prompt or interrupting sign-in. This is a bounded SSH workflow risk suitable for owner awareness and follow-up.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 13 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
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 clearly and concisely describes the primary SSH password dialog fix.
Description check Passed The description is detailed and covers the user impact, mechanism, rationale, testing, scope, and checklist. It provides an issue identifier but does not include an actual issue link or a completed `F…
  • Fix all pre-merge checks with AI
✨ 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
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: a92d9913-b9ff-47c9-bdee-97232cac82f4
📥 Commits

Reviewing files that changed from the base of the PR and between 232f604 and c982f9a.

⛔ Files ignored due to path filters: 1
  • src/shared/rpc-contract/rpc-params-catalog.generated.ts is excluded by !**/*.generated.*
📒 Files selected for processing: 13
  • src/main/ipc/ssh-passphrase.test.ts
  • src/main/ipc/ssh-passphrase.ts
  • src/main/runtime/rpc/methods/ssh.test.ts
  • src/main/runtime/rpc/methods/ssh.ts
  • src/main/ssh/ssh-connection-manager-sign-in-reuse.test.ts
  • src/main/ssh/ssh-connection-manager.test.ts
  • src/main/ssh/ssh-connection-manager.ts
  • src/main/ssh/ssh-connection.ts
  • src/main/ssh/ssh-credential-prompt-login-grace-wire.test.ts
  • src/main/ssh/ssh-target-registry.ts
  • src/renderer/src/web/preload-api/web-terminal-api.ts
  • src/renderer/src/web/web-preload-api-ssh.test.ts
  • src/shared/rpc-contract/ssh-params.ts

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

Comment thread src/main/ssh/ssh-connection-manager.ts Resolved
…ed by a background dial

Co-Authored-By: Claude <noreply@anthropic.com>
@OrcaWin
OrcaWin merged commit d5324c9 into main Oct 15, 2026
39 checks passed
OrcaWin added a commit that referenced this pull request Oct 15, 2026
…ack (#28883)

* fix(ssh): the SSH password dialog no longer closes itself and comes back

A host that drops an unanswered login no longer replaces the open prompt
with a new one, a background dial no longer replaces a connection whose
reconnect is asking the user, one Cancel closes every open prompt for that
host, and a paired client's automatic connect stays automatic.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(ssh): a typed password its reconnect has yet to use is not replaced by a background dial

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: m4air <m4air@Mac.localdomain>
Co-authored-by: Claude <noreply@anthropic.com>
(cherry picked from commit d5324c9)
(cherry picked from commit 4fef196f6de24bcbf5d6a401e31f01b8cd255b86)
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