perf(persistence): stop dead SSH leases pinning metadata, retire unreachable tombstones (#18430)

Two unbounded-growth fixes in the persisted profile, which is re-serialised in
full on every save.

`collectPersistedWorkspaceOwners` registered every SSH lease's worktreeId as a
live persisted owner with no state filter, so a route-retired lease — the
operator-close `terminated` tombstone, or an `expired` row already marked
`supersededBy`/`relayIdRecycled` — pinned its worktree's metadata row
permanently. The prune gate's own doc names that failure: "Rows pinned by a
persisted session are never removable, so the repetition cannot even make
progress." Reuses `sshRemotePtyLeaseAllowsReattach`, the predicate that already
decides which leases still name a route.

`sshRemotePtyLeases` had no pruning path at all: removal happens in three
explicit places, none age- or state-based, so `terminated` rows accumulated
forever (137 rows / 54 KB on the reported profile, ~38/day from one target).
Marking a lease `terminated` scrubs its pane bindings in the same write, so once
no persisted binding names the id the row routes nothing — reattach, pane
recovery, the orphan sweep, `ssh:reset` and `ssh:terminateSessions` all behave
identically on an absent row. Delete it then, gated on that reachability check
because a lease freezes its tabId and the tab-qualified scrub cannot reach a
pane that was detached into a new tab.

`expired` rows are deliberately untouched, superseded ones included:
`sweepOrphanedRelayPtys` reads those ids as its leave-alone list, so dropping one
would authorize stopping a remote shell that supersession left running on
purpose (docs/reference/ssh-execution-boundary.md).
This commit is contained in:
Neil
2026-09-03 20:59:53 -07:00
committed by GitHub
parent ab32c2c0c5
commit c11c6878c1
7 changed files with 324 additions and 25 deletions
@@ -49,14 +49,16 @@ describe('ssh remote pty lease reclaim after a proven reattach', () => {
expect(sshRemotePtyLeaseAllowsReattach(lease)).toBe(true)
})
it('leaves a terminated lease absorbing even when the id appears in a reattach batch', async () => {
it('never lets a reattach batch revive an operator-closed id', async () => {
const store = await createStore()
store.upsertSshRemotePtyLease({ targetId: 'ssh-1', ptyId: 'pty-1', state: 'attached' })
store.markSshRemotePtyLease('ssh-1', 'pty-1', 'terminated')
await store.markSshRemotePtyLeasesAttachedAsync('ssh-1', ['pty-1'])
expect(store.getSshRemotePtyLeases('ssh-1')[0]).toMatchObject({ state: 'terminated' })
// The unbound tombstone is retired at close, and the batch only ever updates existing rows —
// so the id stays out of the reattach set either way.
expect(store.getSshRemotePtyLeases('ssh-1')).toEqual([])
})
it('does not revive an expired lease from an unqualified bulk attach', async () => {
@@ -0,0 +1,147 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { mkdtempSync, rmSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { createStore, testState } from './persistence-test-harness'
import { TEST_LEAF_1 } from './persistence-session-fixtures'
vi.mock('electron', () => ({
app: { getPath: () => testState.dir },
safeStorage: { isEncryptionAvailable: () => false }
}))
vi.mock('./telemetry/client', () => ({ track: vi.fn() }))
vi.mock('./telemetry/cohort-classifier', () => ({ getCohortAtEmit: () => ({}) }))
describe('operator-closed SSH lease tombstones', () => {
beforeEach(() => {
testState.dir = mkdtempSync(join(tmpdir(), 'orca-test-'))
})
afterEach(() => {
rmSync(testState.dir, { recursive: true, force: true })
})
/** A pane whose lease froze `tab-old` before `detachTerminalPaneToTab` moved it to `tab-new`.
* The binding scrub matches tab-qualified, so it cannot reach this row's binding. */
async function storeWithDetachedPaneBinding(): Promise<Awaited<ReturnType<typeof createStore>>> {
const store = await createStore()
store.upsertSshRemotePtyLease({
targetId: 'ssh-1',
ptyId: 'remote-pty',
worktreeId: 'wt1',
tabId: 'tab-old',
leafId: TEST_LEAF_1,
state: 'attached'
})
store.setWorkspaceSession({
activeRepoId: 'r1',
activeWorktreeId: 'wt1',
activeTabId: 'tab-new',
tabsByWorktree: {
wt1: [
{
id: 'tab-new',
worktreeId: 'wt1',
title: 'Terminal',
customTitle: null,
color: null,
sortOrder: 0,
createdAt: 1,
ptyId: null
}
]
},
terminalLayoutsByTabId: {
'tab-new': {
root: { type: 'leaf', leafId: TEST_LEAF_1 },
activeLeafId: TEST_LEAF_1,
expandedLeafId: null,
ptyIdsByLeafId: { [TEST_LEAF_1]: 'ssh:ssh-1@@remote-pty' }
}
}
})
return store
}
it('keeps the tombstone while a binding the scrub could not reach still names the pty', async () => {
const store = await storeWithDetachedPaneBinding()
store.markSshRemotePtyLease('ssh-1', 'ssh:ssh-1@@remote-pty', 'terminated')
// `isRestorablePtyBinding` still consults this row to refuse replaying that binding.
expect(store.getSshRemotePtyLeases('ssh-1')).toEqual([
expect.objectContaining({ ptyId: 'remote-pty', state: 'terminated' })
])
expect(store.getWorkspaceSession().terminalLayoutsByTabId['tab-new'].ptyIdsByLeafId).toEqual({
[TEST_LEAF_1]: 'ssh:ssh-1@@remote-pty'
})
})
it('keeps an operator-closed lease that still owes an undelivered stop', async () => {
const store = await createStore()
store.upsertSshRemotePtyLease({ targetId: 'ssh-1', ptyId: 'remote-pty', state: 'attached' })
store.recordSshRemotePtyKillIntent('ssh-1', 'remote-pty', {
incarnationId: 'inc-1',
requestedAt: 1,
attempts: 0
})
store.markSshRemotePtyLease('ssh-1', 'ssh:ssh-1@@remote-pty', 'terminated')
expect(store.getSshRemotePtyKillIntents('ssh-1', 2)).toHaveLength(1)
expect(store.getSshRemotePtyLeases('ssh-1')).toEqual([
expect.objectContaining({ ptyId: 'remote-pty', state: 'terminated' })
])
})
// `expired` is never evidence the shell died, and `sweepOrphanedRelayPtys` reads these ids as its
// leave-alone list, so dropping one would authorize stopping a process left running on purpose.
it('keeps a superseded expired lease when a sibling pane is closed', async () => {
const store = await createStore()
store.upsertSshRemotePtyLease({
targetId: 'ssh-1',
ptyId: 'remote-pty-1',
worktreeId: 'wt1',
tabId: 'tab1',
leafId: TEST_LEAF_1,
state: 'attached'
})
store.upsertSshRemotePtyLease({
targetId: 'ssh-1',
ptyId: 'remote-pty-2',
worktreeId: 'wt1',
tabId: 'tab1',
leafId: TEST_LEAF_1,
state: 'attached'
})
store.upsertSshRemotePtyLease({ targetId: 'ssh-1', ptyId: 'remote-pty-3', state: 'attached' })
store.markSshRemotePtyLease('ssh-1', 'ssh:ssh-1@@remote-pty-3', 'terminated')
expect(store.getSshRemotePtyLeases('ssh-1')).toEqual([
expect.objectContaining({
ptyId: 'remote-pty-1',
state: 'expired',
supersededBy: 'remote-pty-2'
}),
expect.objectContaining({ ptyId: 'remote-pty-2', state: 'attached' })
])
})
it('retires every unreachable tombstone for the target, not only the one just closed', async () => {
const store = await createStore()
for (const ptyId of ['remote-pty-1', 'remote-pty-2', 'remote-pty-3']) {
store.upsertSshRemotePtyLease({ targetId: 'ssh-1', ptyId, state: 'terminated' })
}
store.upsertSshRemotePtyLease({ targetId: 'ssh-2', ptyId: 'other-pty', state: 'terminated' })
expect(store.getSshRemotePtyLeases()).toHaveLength(4)
store.markSshRemotePtyLease('ssh-1', 'ssh:ssh-1@@remote-pty-1', 'terminated')
// Other targets are untouched: the pass is scoped to the one whose bindings were just scrubbed.
expect(store.getSshRemotePtyLeases()).toEqual([
expect.objectContaining({ targetId: 'ssh-2', ptyId: 'other-pty' })
])
})
})
@@ -526,12 +526,9 @@ describe('Store', () => {
store.markSshRemotePtyLeases('ssh-1', 'terminated')
const session = store.getWorkspaceSession()
expect(store.getSshRemotePtyLeases('ssh-1')).toEqual([
expect.objectContaining({
ptyId: 'remote-pty',
state: 'terminated'
})
])
// The scrub is what retires the row: with no binding left naming the id, the tombstone routes
// nothing and is dropped in the same write.
expect(store.getSshRemotePtyLeases('ssh-1')).toEqual([])
expect(session.tabsByWorktree.wt1[0].ptyId).toBeNull()
expect(session.terminalLayoutsByTabId.tab1.ptyIdsByLeafId).toEqual({})
})
@@ -622,12 +619,8 @@ describe('Store', () => {
store.markSshRemotePtyLease('ssh-1', 'ssh:ssh-1@@remote-pty', 'terminated')
expect(store.getSshRemotePtyLeases('ssh-1')).toEqual([
expect.objectContaining({
ptyId: 'remote-pty',
state: 'terminated'
})
])
// An unresolved id would have left the lease `attached`; this unbound row is retired instead.
expect(store.getSshRemotePtyLeases('ssh-1')).toEqual([])
})
// `expired` never means the shell exited — every writer records that the CLIENT lost its route
@@ -658,12 +651,7 @@ describe('Store', () => {
store.markSshRemotePtyLease('ssh-1', 'ssh:ssh-1@@remote-pty', 'terminated')
const session = store.getWorkspaceSession()
expect(store.getSshRemotePtyLeases('ssh-1')).toEqual([
expect.objectContaining({
ptyId: 'remote-pty',
state: 'terminated'
})
])
expect(store.getSshRemotePtyLeases('ssh-1')).toEqual([])
expect(session.tabsByWorktree.wt1[0].ptyId).toBeNull()
expect(session.terminalLayoutsByTabId.tab1.ptyIdsByLeafId).toEqual({})
})
@@ -2,6 +2,7 @@ import type { PersistedState } from '../../../shared/persisted-state-types'
import type { SshRemotePtyLease } from '../../../shared/ssh-types'
import { isTerminalLeafId } from '../../../shared/stable-pane-id'
import { invalidateLocalWorktreeMetadataPruneInputs } from '../../local-worktree-metadata-prune-gate'
import { pruneRetiredSshRemotePtyLeaseTombstones } from './ssh-pty-lease-tombstone-retention'
import { supersedeSiblingLeasesForPane } from './ssh-pty-pane-supersession'
export type SshPtyLeaseOperations = {
@@ -145,7 +146,11 @@ function updateSshRemotePtyLeaseStates(
const bindingsChanged = shouldClearBindings
? operations.clearBindingsForLeases(targetId, leasesToClear)
: false
return changed || bindingsChanged
// Why after the scrub: it is the scrub that makes the tombstones unreachable.
const tombstonesPruned = shouldClearBindings
? pruneRetiredSshRemotePtyLeaseTombstones(operations, targetId)
: false
return changed || bindingsChanged || tombstonesPruned
}
export function markSshRemotePtyLeases(
@@ -215,10 +220,11 @@ export function markSshRemotePtyLease(
}
const shouldClearBindings = leaseStateWithdrawsBinding(state)
if (lease.state === state) {
if (
(shouldClearBindings && operations.clearBindingsForLeases(targetId, [lease])) ||
recycledChanged
) {
const bindingsCleared =
shouldClearBindings && operations.clearBindingsForLeases(targetId, [lease])
const tombstonesPruned =
shouldClearBindings && pruneRetiredSshRemotePtyLeaseTombstones(operations, targetId)
if (bindingsCleared || tombstonesPruned || recycledChanged) {
operations.flush()
}
return
@@ -233,6 +239,7 @@ export function markSshRemotePtyLease(
}
if (shouldClearBindings) {
operations.clearBindingsForLeases(targetId, [lease])
pruneRetiredSshRemotePtyLeaseTombstones(operations, targetId)
}
operations.flush()
}
@@ -0,0 +1,89 @@
import type { PersistedState } from '../../../shared/persisted-state-types'
import type { SshRemotePtyLease } from '../../../shared/ssh-types'
export type SshPtyLeaseTombstoneRetentionOperations = {
state: PersistedState
toComparablePtyId: (targetId: string, ptyId: string) => string
}
/** A routing tombstone with nothing left to route: the operator closed this PTY and no stop is
* still owed for it. `expired` is deliberately not here — it says only that the CLIENT lost its
* route (docs/reference/ssh-execution-boundary.md), and `sweepOrphanedRelayPtys` reads those ids
* as its leave-alone list, so deleting one would authorize stopping a remote shell that
* supersession left running on purpose. */
function isRetiredRoutingTombstone(lease: SshRemotePtyLease, targetId: string): boolean {
return (
lease.targetId === targetId && lease.state === 'terminated' && lease.pendingKill === undefined
)
}
/** Every stored-form relay pty id some persisted pane binding still names for this target.
*
* Reads all partitions, not only the two `clearSshRemotePtyBindingsForLeases` scrubs: this answer
* authorizes a delete, so a partition left unscanned would be a binding whose tombstone we dropped.
*/
function boundRelayPtyIds(
operations: SshPtyLeaseTombstoneRetentionOperations,
targetId: string
): Set<string> {
const bound = new Set<string>()
const sessions = [
operations.state.workspaceSession,
...Object.values(operations.state.workspaceSessionsByHostId ?? {})
]
for (const session of sessions) {
if (!session) {
continue
}
for (const tabs of Object.values(session.tabsByWorktree ?? {})) {
for (const tab of tabs) {
if (tab.ptyId) {
bound.add(operations.toComparablePtyId(targetId, tab.ptyId))
}
}
}
for (const layout of Object.values(session.terminalLayoutsByTabId ?? {})) {
for (const ptyId of Object.values(layout?.ptyIdsByLeafId ?? {})) {
bound.add(operations.toComparablePtyId(targetId, ptyId))
}
}
}
return bound
}
/**
* Deletes the `terminated` rows nothing can reach, bounding an array that otherwise only grew.
*
* `terminated` is written with a binding scrub in the same call, so once no persisted binding names
* the id the row answers no question any reader asks. Reattach refuses it
* (`sshRemotePtyLeaseAllowsReattach`), pane recovery matches on `expired` only, the orphan sweep
* already classes it neither routed nor expired, and `ssh:reset` / `ssh:terminateSessions` skip it
* outright — every one of those behaves identically on an absent row. The one reader that can still
* observe it is `isRestorablePtyBinding`, and only through a binding whose pty id matches, which is
* exactly what the reachability test rules out. A `pendingKill` is an undelivered stop, so those
* rows stay until the replay retires them.
*
* The reachability test is not redundant with the scrub: a lease freezes its `tabId`, so a pane
* broken out into a new tab leaves a binding the scrub's tab-qualified match no longer reaches.
*
* Does not re-arm the local-worktree-metadata prune gate: a `terminated` lease no longer counts as
* a persisted workspace owner, so dropping one cannot make any metadata row more removable.
*/
export function pruneRetiredSshRemotePtyLeaseTombstones(
operations: SshPtyLeaseTombstoneRetentionOperations,
targetId: string
): boolean {
const leases = operations.state.sshRemotePtyLeases ?? []
if (!leases.some((lease) => isRetiredRoutingTombstone(lease, targetId))) {
return false
}
const bound = boundRelayPtyIds(operations, targetId)
const retained = leases.filter(
(lease) => !isRetiredRoutingTombstone(lease, targetId) || bound.has(lease.ptyId)
)
if (retained.length === leases.length) {
return false
}
operations.state.sshRemotePtyLeases = retained
return true
}
@@ -287,6 +287,64 @@ describe('pruneSessionlessMissingLocalWorktreeMetadataForRepo', () => {
}
})
// A route-retired lease is a tombstone, not a claim: counting one pinned its worktree's metadata
// row for good, so the prune could never make progress on it (#17775).
it('does not let route-retired SSH leases pin a metadata row', () => {
const state = makeState()
const liveIds = Array.from({ length: 3 }, (_, i) => `${REPO_ID}::/workspace/live-${i}`)
const terminatedIds = Array.from({ length: 5 }, (_, i) => `${REPO_ID}::/workspace/closed-${i}`)
const supersededIds = Array.from({ length: 4 }, (_, i) => `${REPO_ID}::/workspace/lost-${i}`)
const recycledIds = [`${REPO_ID}::/workspace/recycled`]
const allIds = [...liveIds, ...terminatedIds, ...supersededIds, ...recycledIds]
for (const worktreeId of allIds) {
state.worktreeMeta[worktreeId] = makeMeta(worktreeId)
}
const lease = (worktreeId: string, index: number, extra: object) => ({
targetId: 'builder',
ptyId: `pty-${index}`,
worktreeId,
createdAt: 1,
updatedAt: 1,
...extra
})
state.sshRemotePtyLeases = [
...liveIds.map((id, i) => lease(id, i, { state: 'detached' })),
...terminatedIds.map((id, i) => lease(id, 100 + i, { state: 'terminated' })),
...supersededIds.map((id, i) =>
lease(id, 200 + i, { state: 'expired', supersededBy: 'pty-9' })
),
...recycledIds.map((id, i) => lease(id, 300 + i, { state: 'expired', relayIdRecycled: true }))
] as never
const scan = capture(state)
expect(pruneCaptured(state, scan, allIds).sort()).toEqual(
[...terminatedIds, ...supersededIds, ...recycledIds].sort()
)
expect(Object.keys(state.worktreeMeta).sort()).toEqual([...liveIds].sort())
})
// A plain `expired` lease says only that the CLIENT lost its route, so its pane is still
// recoverable and its metadata row is still owned (docs/reference/ssh-execution-boundary.md).
it('keeps a metadata row pinned by an unmarked expired lease', () => {
const state = makeState()
const worktreeId = `${REPO_ID}::/workspace/orphaned`
state.worktreeMeta[worktreeId] = makeMeta(worktreeId)
const scan = capture(state)
state.sshRemotePtyLeases = [
{
targetId: 'builder',
ptyId: 'pty',
worktreeId,
state: 'expired',
createdAt: 1,
updatedAt: 1
}
]
expect(pruneCaptured(state, scan, [worktreeId])).toEqual([])
})
it('preserves canonically equivalent session and top-level owners', () => {
const candidateId = `${REPO_ID}::/workspace/Café`.normalize('NFC')
const ownerId = candidateId.normalize('NFD')
@@ -2,6 +2,7 @@ import { isWindowsAbsolutePathLike } from '../../../shared/cross-platform-path'
import { getRepoExecutionHostId, LOCAL_EXECUTION_HOST_ID } from '../../../shared/execution-host'
import type { PersistedState } from '../../../shared/persisted-state-types'
import { getRepoKind } from '../../../shared/repo-kind'
import { sshRemotePtyLeaseAllowsReattach } from '../../../shared/ssh-types'
import { worktreeWorkspaceKey } from '../../../shared/workspace-scope'
import { FOLDER_WORKSPACE_INSTANCE_SEPARATOR, splitWorktreeId } from '../../../shared/worktree/id'
import { isWslUncPath } from '../../../shared/wsl-paths'
@@ -40,6 +41,13 @@ function collectPersistedWorkspaceOwners(
}
}
for (const lease of state.sshRemotePtyLeases) {
// A lease that can never be reattached is a routing tombstone, not a claim on a workspace:
// `terminated` is the operator close, and an `expired` row marked `supersededBy` /
// `relayIdRecycled` already lost its pane to a newer lease. Counting them as owners pinned
// their worktree's metadata row permanently, so the prune could never make progress (#17775).
if (!sshRemotePtyLeaseAllowsReattach(lease)) {
continue
}
add(lease.worktreeId)
}
for (const entry of state.migrationUnsupportedPtyEntries) {