[nwparker]

	Files	Added	Deleted	Net
Test	1	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​32	0	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​32
Prod	1	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​8	$\color{#cf222e}{\Huge{\mathbf{−}}}$​1	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​7

Fixes the relay-reset path recording a verdict it never obtained. Found during the SSH bug sweep (#17817–#17957).

ELI5

ssh:resetRelay bulk-expired every lease on the target inside a finally block — so it fired even when forceStopRelayForTarget threw. A failed stop means we don't know what happened to those shells; marking their leases dead records a conclusion we never reached. Per docs/reference/ssh-execution-boundary.md, loss of contact is never evidence of process death.

Now the expire only runs when the force-stop actually fulfilled:

let relayStopAcknowledged = false
try {
  await forceStopRelayForTarget(conn, targetId)
  relayStopAcknowledged = true
} finally {
  ...
    if (relayStopAcknowledged) {
      persistedStore!.markSshRemotePtyLease(targetId, lease.ptyId, 'expired')
    }

This copies the model already used correctly in the sibling path at ssh-connection-handlers.ts:156, which only marks terminated when provider.shutdown fulfilled and whose comment explicitly keeps the lease alive when shutdown failed.

Local handle cleanup (clearProviderPtyState / deletePtyOwnership), the credential-flag clear, and the disconnect all stay unconditional in the finally — those are stale either way and record no verdict. The error still propagates to the IPC caller.

Nothing is stranded by leaving the leases alone

The next connect resolves them on evidence: ssh-relay-session.ts:2274-2276 and :1982-1987 both filter state !== 'terminated' && state !== 'expired' and attempt reattach.

• If the relay really died, reattach fails not-found and handlePtyReattachFailure expires them.
• If it survived — the force-stop threw before killing anything — the terminals come back, which is the correct outcome for a failed reset.

ssh:terminateSessions also still reaches them.

User-facing regressions

None expected. The only behavior change is on the throw path, where the previous behavior was to record a false verdict. A successful reset is unchanged.

Negative controls: the pre-existing 'ssh:resetRelay force-stops the remote relay and expires tracked leases' and 'ssh:resetRelay clears scoped live PTYs while expiring raw leases' both still pass. The new test also asserts local handles are still cleared on the throw path.

Old-client impact: none. Main-process only — no RPC params, stream frames, or published content changed.

Testing

New test ssh-relay-reset-resume.test.ts → 'ssh:resetRelay keeps leases alive when the force-stop never reported a result'.

Before:

AssertionError: expected "vi.fn()" to not be called at all, but actually been called 2 times
  1st vi.fn() call: [ "ssh-1", "pty-1", "expired" ]
  2nd vi.fn() call: [ "ssh-1", "pty-2", "expired" ]
 Test Files  1 failed (1)
      Tests  1 failed | 13 passed (14)

After:

 Test Files  1 passed (1)
      Tests  14 passed (14)

Suites: 5 files / 50 tests and 3 files / 43 tests green. tc:node, tc:web clean; check:code-quality:changed 0 new findings.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Team

Run ID: 9d101ac4-353b-48fd-97f0-9b68f54546fc

📥 Commits

Reviewing files that changed from the base of the PR and between 5aa02ea and 57baf64.

📒 Files selected for processing (2)

• src/main/ipc/ssh-connection-handlers.ts
• src/main/ipc/ssh-relay-reset-resume.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

The SSH relay reset flow now records whether remote relay force-stop succeeds. It expires persisted remote PTY leases only after successful force-stop completion. When force-stop fails, the error propagates, leases remain unchanged, local PTY state and ownership are cleared, and the connection disconnects. A test covers the channel closed failure case.

Merge Risk: ⚪ Minimal · up to 57baf

This change keeps SSH leases available for recovery when a relay stop fails, preventing live sessions from being incorrectly marked expired. The successful-reset path is unchanged, and no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description explains the change, rationale, behavior, regression test, and test results. However, it omits several required template sections, including an explicit Linked Issue entry using `Fixes…	Add the missing template sections. Provide a linked issue in the required Fixes #<issue> format, state N/A with a reason for Visual Proof if applicable, complete the Review, Agent skill upstream boundary, Notes, and Checklist sections, …
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely identifies the main change: preventing relay-reset leases from expiring when the force-stop throws.
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 explains the change, rationale, behavior, regression test, and test results. However, it omits several required template sections, including an explicit Linked Issue entry using Fixes #, Visual Proof or N/A with a reason, Review, Agent skill upstream boundary, Notes, and the Checklist confirmations.

Resolution

Add the missing template sections. Provide a linked issue in the required Fixes #<issue> format, state N/A with a reason for Visual Proof if applicable, complete the Review, Agent skill upstream boundary, Notes, and Checklist sections, and retain the existing testing details.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[pullfrog]

✅ No new issues found.

Reviewed changes

• Gate lease expiry on force-stop acknowledgment — doResetRelay now marks remote PTY leases expired only when forceStopRelayForTarget fulfilled, so a thrown stop no longer records a verdict the app never obtained.
• Throw-path test coverage — a new ssh:resetRelay test asserts leases stay alive when the force-stop rejects, while local handle cleanup, the credential-flag clear, and disconnect still run unconditionally.

  ^{｜ View workflow run ｜ Using DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏}