refactor(ssh): one home for the exit-proof rule, and comments the house style allows

AGENTS.md asks for brief non-obvious comments, one line where possible. Several
of mine ran to five and eight lines of narrative on the relay's attach path,
which is the one place a reviewer most needs to scan the branching quickly. They
now say the same thing shorter.

Both gone-paths in attach() had independently spelled out the rule that proof
must name the caller's own shell. That duplication is what let the earlier fix
close one and miss the other, so the condition is now a single predicate both
ask — a change to the rule cannot reach one path and skip the other.

The renderer kept its own copy of SSH_SESSION_EXPIRED while this branch created
a shared home for exactly that token, whose whole reason for existing is that the
two copies once disagreed about an identity mismatch and the renderer respawned a
live shell. It imports the shared one now.

Not changed, after checking: the per-generation recovery budget still does not
reset on a successful reattach. Resetting it looks obviously right and is wrong —
a flapping PTY alternates failure and success, and a covering test drives exactly
that for forty rounds. The park cooldown already bounds the harm.

src/main/providers/ssh-pty-provider.ts

@@ -187,12 +187,9 @@ export class SshPtyProvider implements IPtyProvider {
     expectedIncarnationId?: string
   ): Promise<SshPtyAttachResult> {
     // Why: reconnect owns replay delivery so stale/duplicate attach results can
-    // be filtered before they reach the renderer. Pane identity is deliberately
-    // not sent — see reattachSshPtySession; the relay's copy is frozen at spawn,
-    // so it rejected panes that had merely moved tabs.
-    // The shell's own identity IS sent when the caller knows it. This is the path that reconnects
-    // every known pty after a relay comes back, which is exactly when ids have been recycled, so
-    // leaving it unfenced would leave the main reconnect open to attaching somebody else's shell.
+    // be filtered before they reach the renderer. Pane identity stays unsent (the relay froze it at
+    // spawn, so it rejected moved panes); the shell's own identity is sent instead. This path
+    // reattaches every pty after a relay returns — exactly when ids have been recycled.
     const params = {
       id: this.toRelayPtyId(id),
       suppressReplayNotification: true,

src/relay/pty-handler.ts

@@ -336,13 +336,10 @@ export type PtyExitListener = (event: { id: string; paneKey?: string }) => void
 
 type PtyIdentity = { paneKey?: string; tabId?: string }
 
-/* Why there is no attach-time pane-identity comparison here any more: it keyed on the paneKey and
- * tabId frozen at spawn, so moving a pane to another tab made the relay refuse its own live shell,
- * and refuse by saying "not found" — which the client read as death and answered by resuming the
- * agent a second time. Cross-generation id collisions, the thing it was actually for, are rejected
- * by the incarnation comparison in `attach` instead: that is the shell's own identity, so it
- * follows the pane wherever it moves. `attachIdentity` is still recorded and still round-trips
- * through serialize/revive; it just no longer gates an attach. */
+/* No attach-time pane-identity comparison any more: it froze paneKey/tabId at spawn, so a moved
+ * pane was refused its own live shell — as "not found", which read as death. Recycled ids, its real
+ * purpose, are caught by the incarnation comparison in `attach` instead. `attachIdentity` is still
+ * recorded and round-trips through serialize/revive; it no longer gates an attach. */
 /** Returns env to merge into the PTY's spawn env. Receives spawn context so augmenters can derive per-PTY identity from paneKey.
  *  `command` is the renderer-chosen agent launch command (`pi`, `omp`, …); undefined for CLI-launched bare shells. */
 export type PtyEnvAugmenter = (ctx: {
@@ -367,13 +364,9 @@ export class PtyHandler {
   private outputFlushTimer: ReturnType<typeof setTimeout> | null = null
   private pendingOutputByPty = new Map<string, PendingPtyOutput[]>()
   private pendingExitByPty = new Map<string, { id: string; code: number; incarnationId: string }>()
-  /**
-   * Shells this process WATCHED exit, kept after the exit is published. Attach can then answer
-   * "it exited" instead of "not found", and only that first-hand answer is proof: a relay that
-   * merely never knew the id may be a replacement whose predecessor's shells are still running.
-   * Bounded and oldest-evicted — losing one costs a respawn the user has to ask for, never a wrong
-   * one. A crash loses them all, which correctly reads as no knowledge.
-   */
+  /** Exits this process WATCHED, kept past publication so attach can prove death rather than report
+   *  an unknown id. Bounded and oldest-evicted; losing one costs a user-asked respawn, never a
+   *  wrong one, and a crash losing all of them correctly reads as no knowledge. */
   private exitedPtys = new Map<string, { code: number; incarnationId: string }>()
   private pausedOutputPtys = new Set<string>()
   private consumerPausedOutputPtys = new Set<string>()
@@ -754,6 +747,16 @@ export class PtyHandler {
     })
   }
 
+  /** Proof must name the caller's own shell: present and matching. Both gone-paths ask this, so a
+   *  change to the rule cannot reach one and miss the other. */
+  private exitProofAnswersCaller(
+    expectedIncarnationId: string | undefined,
+    exitedIncarnationId: string,
+    clientUnderstandsExitProof: boolean
+  ): boolean {
+    return clientUnderstandsExitProof && expectedIncarnationId === exitedIncarnationId
+  }
+
   /** Oldest-evicted: a Map iterates in insertion order, so the first key is the oldest. */
   private rememberPtyExit(id: string, code: number, incarnationId: string): void {
     this.exitedPtys.delete(id)
@@ -1619,30 +1622,23 @@ export class PtyHandler {
     const id = params.id as string
     const expectedIncarnationId =
       typeof params.expectedIncarnationId === 'string' ? params.expectedIncarnationId : undefined
-    // Why gated: what the host answers reaches clients that predate this reply, and one of those
-    // reads an unrecognized attach error as neither death nor recovery — it strands the pane. A
-    // client that understands the proof says so; everyone else keeps the wording they already act
-    // on. Nothing is lost by staying quiet, because an older client could not have used it.
+    // Gated because the host's answer reaches clients predating it, which read an unrecognized
+    // attach error as neither death nor recovery and strand the pane.
     const clientUnderstandsExitProof = params.exitProofSupported === true
     const managed = this.ptys.get(id)
     // Why: after dispose, pty.kill is a POSIX no-op; treat disposed as not-found so failures aren't silent.
     if (!managed || managed.disposed) {
-      // `disposed` is a teardown flag, not an observed exit, so only a genuinely absent pty may be
+      // `disposed` is teardown, not an observed exit, so only a genuinely absent pty may be
       // answered from what this process watched exit.
       if (!managed) {
         const exited = this.exitedPtys.get(id)
-        // A remembered exit for a DIFFERENT shell says nothing about the caller's: answer the
-        // ordinary unknown instead, or a recycled id would report somebody else's death as ours.
-        // The expectation must be PRESENT and match. A caller that names no shell is not asking
-        // about this one, and answering it anyway is how a remembered exit becomes a claim about
-        // somebody else's shell: relay A is killed leaving pty-1 alive and orphaned, relay B mints
-        // its own pty-1 and that one exits, and a pane with no recorded identity would be told its
-        // shell is gone — then replace a process that is still running. A pane with nothing to
-        // compare gets the disconnected card instead, which is the safe direction.
         if (
-          clientUnderstandsExitProof &&
           exited &&
-          expectedIncarnationId === exited.incarnationId
+          this.exitProofAnswersCaller(
+            expectedIncarnationId,
+            exited.incarnationId,
+            clientUnderstandsExitProof
+          )
         ) {
           throw new Error(formatPtyExitedError(id, exited.code, exited.incarnationId))
         }
@@ -1660,19 +1656,19 @@ export class PtyHandler {
       disposeManagedPty(managed)
       this.removePty(id)
       this.clearPtyFlowState(id)
-      // First-hand proof: this process held the shell and its pid is gone. Saying "not found"
-      // here throws away the one observation that distinguishes a dead shell from an unknown one.
-      // The reap above happens either way — a dead shell should be cleared no matter who asked —
-      // but the ANSWER still depends on whose shell it was. Under this id a replacement relay may
-      // hold a different shell entirely, and its death says nothing about the caller's, which may
-      // be running orphaned under the relay this one replaced. Same rule as the remembered exit:
-      // proof needs a present, matching expectation.
+      // The reap runs whoever asked, but the ANSWER depends on whose shell died: under this id a
+      // replacement relay may hold a different one entirely, and the caller's may be orphaned and
+      // alive under the relay this one replaced.
       this.rememberPtyExit(id, PTY_EXIT_CODE_OBSERVED_GONE, managed.incarnationId)
       if (expectedIncarnationId && expectedIncarnationId !== managed.incarnationId) {
         throw new Error(`PTY "${id}" identity mismatch`)
       }
       throw new Error(
-        clientUnderstandsExitProof && expectedIncarnationId === managed.incarnationId
+        this.exitProofAnswersCaller(
+          expectedIncarnationId,
+          managed.incarnationId,
+          clientUnderstandsExitProof
+        )
           ? formatPtyExitedError(id, PTY_EXIT_CODE_OBSERVED_GONE, managed.incarnationId)
           : `PTY "${id}" not found`
       )

src/renderer/src/components/terminal-pane/pty-connection.ts

@@ -13,6 +13,7 @@ import { scheduleRuntimeGraphSync } from '@/runtime/sync-runtime-graph'
 import { useAppStore } from '@/store'
 import { getWorktreeMapFromState } from '@/store/selectors'
 import { parseWorkspaceKey } from '../../../../shared/workspace-scope'
+import { SSH_SESSION_EXPIRED_ERROR } from '../../../../shared/ssh-pty-failure-tokens'
 import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants'
 import { isEphemeralSetupTerminalWorktreeId } from '../../../../shared/ephemeral-setup-terminal-worktree-id'
 import { TERMINAL_PAIRED_PARKING_RUNTIME_CAPABILITY } from '../../../../shared/protocol-version'
@@ -341,7 +342,7 @@ import { directSshAuthoritiesEqual } from '@/store/slices/direct-ssh-terminal-au
 import { isProvenSshSessionGoneError } from './reattach-failure-classification'
 
 const pendingSpawnByPaneKey = new Map<string, Promise<string | null>>()
-const SSH_SESSION_EXPIRED_ERROR = 'SSH_SESSION_EXPIRED'
+
 // Why: relay requests expire at 30s; leave one second for their fallback before re-arming locally.
 const DIRECT_SSH_PANE_RETRY_SETTLEMENT_TIMEOUT_MS = 31_000
 const REMOTE_PTY_ID_PREFIX = 'remote:'

src/renderer/src/components/terminal-pane/pty-transport.ts

@@ -13,6 +13,7 @@ import {
   iterateTerminalInputChunks
 } from '../../../../shared/terminal-input'
 import { isRuntimeOwnedSshTargetId } from '../../../../shared/execution-host'
+import { SSH_SESSION_EXPIRED_ERROR } from '../../../../shared/ssh-pty-failure-tokens'
 import {
   ptyDataHandlers,
   ptyReplayHandlers,
@@ -71,7 +72,6 @@ export type {
 } from './pty-transport-types'
 export { extractLastOscTitle } from '../../../../shared/agent-detection'
 
-const SSH_SESSION_EXPIRED_ERROR = 'SSH_SESSION_EXPIRED'
 // Why: main rejects a session reattached under a different SSH connection with this phrase; treat as stale (spawn fresh), not a crash.
 const SSH_PTY_CONNECTION_MISMATCH_MARKER = 'belongs to SSH connection'
 const STALE_TITLE_TIMEOUT = 3000 // ms before stale working title is cleared

src/shared/ssh-pty-failure-tokens.ts

@@ -14,14 +14,9 @@ export const SSH_PTY_IDENTITY_MISMATCH_ERROR = 'SSH_PTY_IDENTITY_MISMATCH'
 /** The shell is alive; only its output source must be re-established. */
 export const SSH_SOURCE_RESTORE_REQUIRED_ERROR = 'SSH_SOURCE_RESTORE_REQUIRED'
 
-/**
- * The relay WATCHED this shell exit. That is first-hand knowledge from the process that owned it,
- * and the only answer that proves death: a relay which merely never knew the id may be a
- * replacement whose predecessor's shells are still running.
- *
- * Deliberately worded so it cannot be mistaken for the ordinary unknown — the phrase
- * `PTY "<id>" not found` is what an older client maps to expiry, and expiry authorizes a respawn.
- */
+/** The relay WATCHED this exit, so it proves death — unlike an unknown id, which a replacement
+ *  relay also answers for shells still running. Worded to never read as `not found`, which older
+ *  clients map to expiry, and expiry authorizes a respawn. */
 export const SSH_PTY_EXITED_ERROR = 'SSH_PTY_EXITED'
 
 /** The carrier is the message: the relay's error transport drops structured payloads. */
@@ -29,12 +24,8 @@ export function formatPtyExitedError(id: string, code: number, incarnationId: st
   return `${SSH_PTY_EXITED_ERROR}: ${encodeURIComponent(id)} code=${code} incarnation=${encodeURIComponent(incarnationId)}`
 }
 
-/**
- * Anchored on the WHOLE grammar, not the token. This answer authorizes replacing a shell, so a
- * substring test would let any text that merely quotes the token — a log line, a wrapped error, a
- * shell printing it — stand in for the relay's own observation. Ids and incarnations are
- * percent-encoded at the source so neither can contain a space and forge the fields after it.
- */
+/** Whole grammar, not the token: this authorizes replacing a shell, so any text merely quoting the
+ *  token must not qualify. Fields are percent-encoded at the source, so none can forge the next. */
 const SSH_PTY_EXITED_MESSAGE = new RegExp(
   `(?:^|[^A-Z_])${SSH_PTY_EXITED_ERROR}: ([^\\s]+) code=(-?\\d+) incarnation=([^\\s]+)`
 )
@@ -43,11 +34,8 @@ export function isSshPtyExitedMessage(message: string): boolean {
   return SSH_PTY_EXITED_MESSAGE.test(message)
 }
 
-/**
- * The shell the proof is about. A caller must check this against the shell it asked for before
- * acting: the host enforces the same rule, but the host is the party whose answer is in question,
- * and versions differ. An unverifiable proof is simply not proof.
- */
+/** The shell the proof names. Callers must check it themselves: the host applies the same rule, but
+ *  the host is the party whose answer is in question and versions differ. */
 export function parsePtyExitedError(
   message: string
 ): { id: string; code: number; incarnationId: string } | null {