[nwparker]

	Files	Added	Deleted	Net
Test	4	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​121	$\color{#cf222e}{\Huge{\mathbf{−}}}$​9	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​112
Prod	3	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​42	$\color{#cf222e}{\Huge{\mathbf{−}}}$​9	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​33

Follow-up to the six-PR lease stack (#17951, #17957, #17965, #17966, #17969, #18013), which split the meaning of expired: SshRemotePtyLeaseState has no unverifiable member, so "we don't know" is persisted as expired, and an expired lease is an orphan unless a mark says its route died for good. sshRemotePtyLeaseAllowsReattach is that rule.

Three main-side readers still tested state === 'expired' directly. They do not all want the same answer — this audits each one and migrates only the two that were asking the reattach question.

Migrated

ssh:terminateSessions reconnect fence (src/main/ipc/ssh-connection-handlers.ts). An expired lease carrying no retirement mark records only that reattach gave up, so it is exactly the orphan the user's "End Remote Terminals" has to reach — and reaching it needs the relay. It now counts as owned and raises SSH_TERMINATE_RECONNECT_REQUIRED, which the renderer answers by connecting and retrying (terminateSshSessionsWithReconnect). supersededBy / relayIdRecycled leases stay unowned and keep being swept opportunistically, which is what preserves #2626: a target that can no longer answer never blocks its own removal (removeSshTargetWithBestEffortCleanup already tolerates the refusal).

Kill-intent row disposal (src/main/persistence/leasing-ssh-ptys/ssh-pty-kill-intent-operations.ts). isDisposableKillOnlyLease collected any pane-less terminated or expired row once its pendingKill was retired. For an unmarked expired lease that row is the client's last route back to a shell that may still be running: deleting it takes the id out of the bulk reattach set (reattachKnownPtys) and out of the orphan sweep's leave-alone list in one write, turning a process left running on purpose into a sweepable one. It also contradicted the TTL path's own comment that an order aging out "observes nothing about the process". Now gated on !sshRemotePtyLeaseAllowsReattach, so terminated and marked-expired rows are still collected.

Kept raw, with the reason recorded

doResetRelay's bulk expire. This asks which routes the force-stop just invalidated, not which leases may be reattached. An already-expired lease has no route left to retire: markSshRemotePtyLease early-returns on an unchanged state, and any local handle this connection really holds arrives through getPtyIdsForConnection, so migrating here is a strict no-op. Reset also must not upgrade it to terminated — killing the relay daemon makes its PTYs permanently unreachable, which is not evidence they exited (docs/reference/ssh-execution-boundary.md). Nothing can adopt a stranger either: the replacement relay namespaces every id under a fresh ptyIdMintEpoch, so an old orphan lease can only fail its next reattach.

ssh-orphan-relay-pty-sweep.ts's clientClaims. Precedent here is terminal-binding-recovery.ts, which deliberately uses a different predicate because it asks a different question. This one asks "may this client stop the process", and every non-terminated state answers no either way — routed and expiredLeasePtyIds are both skips, so sorting the marked leases into routed would move nothing but the skip-reason string. Both marks are written by paths that leave the remote process running on purpose, so they must keep refusing the stop rather than authorizing one.

Tests

Two behavior changes, two tests that fail without the source edit (verified by stashing only the source files):

• ssh:terminateSessions demands a reconnect for an unmarked expired lease
• keeps an unmarked expired lease when its intent ages out

Plus coverage for the leases the predicate excludes and the raw check does not, since those are where the two diverge: superseded and relayIdRecycled cases on the terminate fence, both marks on the kill-intent disposal, relayIdRecycled on the sweep, and all three expired shapes pinned as untouched by reset. The pre-existing #12661 test now drives a superseded lease, keeping the "no forced reconnect for a route that died for good" case on the lease that actually has that property.

Verified: pnpm tc, pnpm test src, pnpm run check:code-quality:changed, pnpm test:e2e:ssh-docker.

[coderabbitai]

📝 Walkthrough

Walkthrough

SSH lease handling now distinguishes unmarked expired leases from superseded or recycled leases. Termination can require reconnects for unmarked expired leases. Kill-intent pruning retains those leases while removing retired routes. Reset tests cover all expired lease variants. Orphan relay sweep coverage confirms that recycled relay IDs do not stop the corresponding PTY.

Merge Risk: 🟠 High · up to 5f75c

Terminating SSH sessions can kill a newer active PTY that reused a recycled relay ID. This should be fixed before merge.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description provides detailed change rationale and test results, but it does not follow the repository template. It omits the required Linked Issue entry, Visual Proof section with an N/A justific…	Update the description to include all required template sections. Add a linked issue using the required Fixes #... format, add the Visual Proof section with N/A and a reason because this is not a visual change, complete the Testing chec…
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 7 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 describes the main change: routing remaining expired-lease readers through the reattach predicate.
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 provides detailed change rationale and test results, but it does not follow the repository template. It omits the required Linked Issue entry, Visual Proof section with an N/A justification, checklist confirmations, and several template headings.

Resolution

Update the description to include all required template sections. Add a linked issue using the required Fixes #... format, add the Visual Proof section with N/A and a reason because this is not a visual change, complete the Testing checkboxes and platform details, and address the remaining checklist and boundary sections.

• 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.}

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Team

Run ID: cd070698-562a-4f84-b9f8-892add75bd1e

📥 Commits

Reviewing files that changed from the base of the PR and between 573537e and 5f75c94.

📒 Files selected for processing (7)

• src/main/ipc/ssh-connection-handlers.ts
• src/main/ipc/ssh-relay-reset-resume.test.ts
• src/main/ipc/ssh-terminate-sessions.test.ts
• src/main/persistence-ssh-pending-pty-kill.test.ts
• src/main/persistence/leasing-ssh-ptys/ssh-pty-kill-intent-operations.ts
• src/main/ssh/ssh-orphan-relay-pty-sweep.test.ts
• src/main/ssh/ssh-orphan-relay-pty-sweep.ts

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

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Exclude retired leases from provider shutdown candidates. In ssh:terminateSessions, sshRemotePtyLeaseAllowsReattach(lease) returns false for an expired lease with relayIdRecycled, but trackPtyId still adds its ID to ptyIds. SshPtyProvider.shutdown sends that relay ID without expectedIncarnationId, so pty.shutdown can kill the newer PTY using the recycled ID. Skip leases that fail the predicate when building shutdown candidates, while preserving their unverifiable accounting.

[pullfrog]

✅ No new issues found.

Reviewed changes

• ssh:terminateSessions reconnect fence — unmarked expired leases now count as owned and raise SSH_TERMINATE_RECONNECT_REQUIRED when no provider is attached; supersededBy / relayIdRecycled leases stay unowned.
• Kill-intent row disposal — isDisposableKillOnlyLease now gates on !sshRemotePtyLeaseAllowsReattach, retaining unmarked expired rows while still collecting terminated and marked-expired rows.
• Documentation-only — doResetRelay's bulk expire and the orphan sweep's clientClaims keep their raw state checks, with the reasons recorded.
• Tests — two tests that fail without the source edit, plus characterization coverage for the divergent cases.

The two behavior changes are narrow and consistent: the shutdown list in terminateSessions is unchanged (only the reconnect fence flag moves), and isDisposableKillOnlyLease differs from the old condition solely on the unmarked-expired case. The predicate is already the rule reattachKnownPtys and the pane-recovery reader use, and removeSshTargetWithBestEffortCleanup tolerates the reconnect refusal, so #2626 stays preserved.

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