Files
orca/src/shared/ssh-pending-pty-kill.ts
NeilandBrennan Benson fbe94ceff6 fix: close readiness gaps found by merged-change audit (#17159)
* fix(ssh): fence stale kills and retired pane replay

* fix(ssh): support cancellable interactive authentication

* fix(ssh): await remote catalog before snapshot adoption

* fix(pty): contain Windows ConPTY input failures

* fix(power): avoid redundant macOS display blocking

* perf(editor): narrow markdown override subscriptions

* fix(quick-open): close directory handles after reads

* refactor(linux): remove unused proc socket scanner

* fix(usage): apply flat Sonnet 4.6 pricing

* ci: prime Node next native test cache

* docs(skills): resolve snapshot cleanup data path

* fix(ssh): recover install locks after host reboot

* test(ssh): recognize boot-aware install locks

* test(ssh): prove previous-boot lock recovery live

* test(wire): pin pre-metadata release coverage

* fix(terminal): preserve remote tab ownership through recovery races

* test(runtime): fence replaced terminal handles in agent guard

* fix(ssh): preserve remote snapshot authority across polls

* fix(pty): contain late ConPTY output EPIPE

* test(pty): register Windows exit watcher before kill

* fix: close SSH and tab readiness race gaps

* fix(tabs): retain headless order and placeholder titles

* fix(build): avoid parallel electron-vite config race

* test(windows): avoid MSYS temp path rewriting

* test(windows): avoid killing exited PTY

* fix(pty): avoid late ConPTY input teardown race

* fix(terminal): sync reconnect error ownership after commit

* fix(runtime): use canonical worktree identity comparison

* test(ssh): assert complete cold-hydration baseline

* test(windows): invoke quoted retention fixture via PowerShell

* test(windows): read ConPTY grid through mode con

* fix(terminal): publish PTY replacements atomically

* fix(terminal): infer stale identity on reattach

* fix(terminal): fence stale pane PTY callbacks

* fix(terminal): fence stale pane binds after rebind

* fix(terminal): reject stale pane transport callbacks

* fix(terminal): fence mirrored reattach spawn callbacks

* fix(terminal): replace stale pane PTYs on remount

* fix(ci): size the Windows launcher-compile test budget from measurement

`native-smoke (windows-latest)` fails ~4.5% of runs on
`preserves a multiline argument through the compiled remote launcher`
with "Test timed out in 15000ms" — on unrelated PRs, for reasons that
have nothing to do with them. Across 176 sampled attempts it is the only
red that job produced, and it hit seven different PRs in two days:
#16900, #16904, #16915, #16955 (twice), #16979, #17014, #17085.

The test is six process creations: powershell.exe forks csc.exe, then
the freshly compiled orca.exe forks node.exe, twice. Hosted Windows
runners periodically slow process creation down, and this test amplifies
that far harder than anything else in the job. Comparing the 80 attempts
where it ran under 3s against the 12 where it ran over 12s, its own
median goes 2198ms -> 15917ms (7.2x) while the same file's
powershell-only test moves 556 -> 686ms (1.2x), the cmd.exe and Git Bash
process tests in the neighbouring file move 1.4x, and the other 35 files
put together move 1.5x.

Measured across those 176 attempts: 1881ms to 35438ms, p50 4264ms,
correlation +0.881 with the job's total Vitest duration. 8 of 176 (4.5%)
exceeded the 15s cap; 2 of 176 (1.1%) also exceeded the shared 30s
testTimeout, so deleting the override and inheriting the config is not
enough on its own. 60s clears all 176 with 1.7x headroom on the worst.

This is slow, not hung. Every body here is synchronous spawnSync, so
Vitest cannot interrupt one — the timer fires only after the body
returns and the reported duration is real elapsed time. That is why a
failure reads `× ... 22464ms` under `Test timed out in 15000ms`. The
work finished; the stopwatch was short. Seven reruns at one identical
head measured 2053 / 4680 / 5551 / 8732 / 13506 / 14868 / 21937ms — the
last of those would have been red on code that had not changed.

The 15s came from #8897, which raised this test off Vitest's built-in 5s
default because the job then ran bare `pnpm vitest run`. #8909 landed
3h27m later and pointed the job at config/vitest.config.ts, which is the
real fix for that. The constant stayed behind and has been the binding
budget ever since.

* fix(terminal): fence stale remount reattach ownership

* fix(terminal): reconcile mounted pane identity after replacement

* fix(terminal): fence stale reattach fallback ownership

* fix(terminal): fence deferred SSH reattach ownership

* fix(terminal): fence stale split pane ownership callbacks

* fix(terminal): keep stale spawns from consuming startup

---------

Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com>
2026-08-31 08:17:40 -07:00

148 lines
7.0 KiB
TypeScript

import type { SshRemotePtyLease } from './ssh-types'
/** A `pty.shutdown` this client issued to an SSH host and could not confirm.
*
* Why it must exist: a relay PTY is a child of the detached relay daemon, not of the ssh channel,
* so a shutdown that dies on the transport leaves a live shell — and often a live agent — running
* on the user's remote machine with nothing left to retry it. Marking the attempt `unverifiable`
* is correct but is not a fix; the kill is an intent that has to outlive the transport failure and
* be replayed against the authoritative host when contact returns.
*
* Carried on the existing `SshRemotePtyLease` rather than in a second journal: the lease is
* already the durable, restart-surviving, per-`(targetId, relayPtyId)` record of a remote PTY. */
export type SshPendingPtyKill = {
requestedAt: number
/** The host-minted PTY incarnation this kill was aimed at, and the whole fence.
*
* A relay renumbers from `pty-1` on every start, so `(targetId, relayPtyId)` alone can name a
* DIFFERENT shell after a redeploy — the collision behind #16970. Current relays enforce this
* identity on `pty.shutdown`; the client also proves it from `pty.listProcesses` before replay so
* older relays that ignore the additive field keep the safest available fallback. */
incarnationId: string
/** Replays attempted since. Diagnostic; the TTL, not this, is the bound. */
attempts: number
}
/** Colocated with the type so the two cannot drift. The lease loader is a strict whitelist that
* drops anything it does not name, so a record omitted here would be silently stripped on every
* launch — the exact failure `closed-terminal-tab-tombstones.ts` records having shipped once. */
export function normalizeSshPendingPtyKill(value: unknown): SshPendingPtyKill | null {
if (!value || typeof value !== 'object') {
return null
}
const raw = value as Partial<SshPendingPtyKill>
if (typeof raw.requestedAt !== 'number' || !Number.isFinite(raw.requestedAt)) {
return null
}
// No incarnation, no fence, and an unfenced kill order is worse than none.
if (
typeof raw.incarnationId !== 'string' ||
!raw.incarnationId ||
raw.incarnationId.length > 128
) {
return null
}
return {
requestedAt: raw.requestedAt,
incarnationId: raw.incarnationId,
attempts: typeof raw.attempts === 'number' && raw.attempts >= 0 ? raw.attempts : 0
}
}
/** Backstop only — host acknowledgement is the normal exit. This covers a target the user never
* reconnects to, whose intent would otherwise be carried forever. Deliberately longer than the 7d
* ceiling on a relay grace window, so the intent outlives the longest window in which the host
* could still be holding the PTY it names. */
export const SSH_PENDING_PTY_KILL_TTL_MS = 30 * 24 * 60 * 60 * 1000
/** Per target, newest kept. A permanently unreachable host must not grow the store without bound. */
export const MAX_SSH_PENDING_PTY_KILLS_PER_TARGET = 200
/** The single definition of "too old to act on". Both the durable prune and the decision function
* read it, so a stale order cannot be replayed by one and kept by the other. */
export function isSshPendingPtyKillExpired(intent: SshPendingPtyKill, now: number): boolean {
return now - intent.requestedAt > SSH_PENDING_PTY_KILL_TTL_MS
}
/** What the authoritative host just said about this relay PTY id, from one `pty.listProcesses`. */
export type SshPendingPtyKillObservation = {
/** False only when the host answered and did not list the id: positive evidence of absence.
* A failed or timed-out listing is never expressed here — the caller defers instead. */
hostListsPty: boolean
/** The incarnation the host published for that id. `undefined` means the host published none,
* which reads as unknown — never as "no incarnation" and never as a match. */
hostIncarnationId: string | undefined
}
export type SshPendingPtyKillRetirement =
| 'host-reports-absent'
| 'relay-id-recycled'
| 'stop-confirmed'
export type SshPendingPtyKillDecision =
| { action: 'replay' }
| { action: 'retire'; reason: Exclude<SshPendingPtyKillRetirement, 'stop-confirmed'> }
| { action: 'defer'; reason: string }
/** Decides what to do with one recorded kill, given what the host just said about its id.
*
* The TTL is deliberately NOT a branch here. `isSshPendingPtyKillExpired` owns it, applied as a
* durable prune before any of this runs, so expired orders are actually deleted from the store
* rather than merely skipped — and so there is exactly one place that decides what "too old"
* means. A branch here would be unreachable behind that prune and would only look tested.
*
* `defer` is the `unverifiable` branch and asserts nothing: the record stays and the next
* handshake asks again. No branch concludes that a PTY exited — only `host-reports-absent` is an
* observation of absence, and it comes from the host that owns the process. */
export function decideSshPendingPtyKill(
intent: SshPendingPtyKill,
observation: SshPendingPtyKillObservation,
now: number
): SshPendingPtyKillDecision {
if (isSshPendingPtyKillExpired(intent, now)) {
// Unreachable behind the prune, but a stale order must never be aimed at whatever holds the
// id today if a future caller reaches this without pruning first.
return { action: 'defer', reason: 'order is past its TTL and awaiting prune' }
}
if (!observation.hostListsPty) {
return { action: 'retire', reason: 'host-reports-absent' }
}
if (observation.hostIncarnationId === undefined) {
// Why not replay: an unfenced kill against a renumbered relay id destroys a shell nobody asked
// to close, which is strictly worse than the leak. Hosts predating the published incarnation
// therefore degrade to no replay rather than to a guess.
return { action: 'defer', reason: 'host published no PTY incarnation for this id' }
}
if (observation.hostIncarnationId !== intent.incarnationId) {
return { action: 'retire', reason: 'relay-id-recycled' }
}
return { action: 'replay' }
}
export type SshPendingPtyKillEntry = { ptyId: string; intent: SshPendingPtyKill }
/** Newest-first, TTL-filtered and capped — the same shape as `pruneClosedTerminalTabTombstones`. */
export function prunePendingSshPtyKills(
entries: readonly SshPendingPtyKillEntry[],
now: number
): SshPendingPtyKillEntry[] {
return entries
.filter((entry) => !isSshPendingPtyKillExpired(entry.intent, now))
.sort((a, b) => b.intent.requestedAt - a.intent.requestedAt)
.slice(0, MAX_SSH_PENDING_PTY_KILLS_PER_TARGET)
}
/** Reads across every lease state on purpose: a kill issued while the provider was already gone
* tombstones its lease `terminated` and still leaves the remote process running. */
export function pendingSshPtyKillEntries(
leases: readonly SshRemotePtyLease[]
): SshPendingPtyKillEntry[] {
const entries: SshPendingPtyKillEntry[] = []
for (const lease of leases) {
if (lease.pendingKill) {
entries.push({ ptyId: lease.ptyId, intent: lease.pendingKill })
}
}
return entries
}