mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 16:03:20 +00:00
fix(ssh): two lease-retirement primitives, split by evidence (STA-3077, part 2/5)
Every future retirement goes through exactly one of two Store methods, split by what the evidence actually proves rather than by which handler produced it: - retireLeaseAndReap — the pane is provably gone, so the remote PTY is ours to kill. Sets pendingRemoteShutdown. - retireLeaseSparingPty — the row is wrong about the PTY it names. Cannot reach pendingRemoteShutdown by construction, because a reset relay reuses pty-N and the id may already be another pane's live shell. Retirement is terminal: re-retiring is refused, so a spared lease can never later acquire a reap flag. Both retirement fields are added to the type and to normalizeSshRemotePtyLease here rather than in part 3. That normalizer is a strict allowlist, so a field landing without it would work in every in-process test and vanish on the first restart — precisely the durable case the flag exists for. The round-trip test reloads through normalize instead of set-then-read. No callers yet; this commit only constrains what comes after.
This commit is contained in:
@@ -9386,6 +9386,114 @@ describe('Store', () => {
|
||||
}
|
||||
})
|
||||
|
||||
it('retires and flags for reaping when the evidence is that the pane is gone', async () => {
|
||||
const store = await createStore()
|
||||
store.upsertSshRemotePtyLease({
|
||||
targetId: 'target-1',
|
||||
ptyId: 'pty-1',
|
||||
worktreeId: 'wt1',
|
||||
tabId: 'tab-1',
|
||||
leafId: TEST_LEAF_1,
|
||||
state: 'attached'
|
||||
})
|
||||
|
||||
store.retireLeaseAndReap('target-1', 'pty-1', 'superseded by a later write for the same pane')
|
||||
|
||||
const lease = store.getSshRemotePtyLeases('target-1')[0]
|
||||
expect(lease.state).toBe('terminated')
|
||||
expect(lease.retiredReason).toBe('superseded by a later write for the same pane')
|
||||
expect(lease.pendingRemoteShutdown).toBe(true)
|
||||
})
|
||||
|
||||
it('retires without ever flagging a reap when the evidence is only that the row is wrong', async () => {
|
||||
const store = await createStore()
|
||||
store.upsertSshRemotePtyLease({
|
||||
targetId: 'target-1',
|
||||
ptyId: 'pty-1',
|
||||
worktreeId: 'wt1',
|
||||
tabId: 'tab-1',
|
||||
leafId: TEST_LEAF_1,
|
||||
state: 'attached'
|
||||
})
|
||||
|
||||
store.retireLeaseSparingPty('target-1', 'pty-1', 'relay generation replaced')
|
||||
|
||||
const lease = store.getSshRemotePtyLeases('target-1')[0]
|
||||
expect(lease.state).toBe('expired')
|
||||
expect(lease.retiredReason).toBe('relay generation replaced')
|
||||
// A reset relay reuses `pty-N`, so this id may already be another pane's live shell.
|
||||
expect(lease.pendingRemoteShutdown).toBeUndefined()
|
||||
})
|
||||
|
||||
it('refuses to re-retire, so a spared lease can never later acquire a reap flag', async () => {
|
||||
const store = await createStore()
|
||||
store.upsertSshRemotePtyLease({
|
||||
targetId: 'target-1',
|
||||
ptyId: 'pty-1',
|
||||
worktreeId: 'wt1',
|
||||
tabId: 'tab-1',
|
||||
leafId: TEST_LEAF_1,
|
||||
state: 'attached'
|
||||
})
|
||||
|
||||
store.retireLeaseSparingPty('target-1', 'pty-1', 'relay generation replaced')
|
||||
store.retireLeaseAndReap('target-1', 'pty-1', 'pane closed')
|
||||
|
||||
const lease = store.getSshRemotePtyLeases('target-1')[0]
|
||||
expect(lease.state).toBe('expired')
|
||||
expect(lease.retiredReason).toBe('relay generation replaced')
|
||||
expect(lease.pendingRemoteShutdown).toBeUndefined()
|
||||
})
|
||||
|
||||
it('round-trips retirement fields through normalize on restart', async () => {
|
||||
const store = await createStore()
|
||||
store.upsertSshRemotePtyLease({
|
||||
targetId: 'target-1',
|
||||
ptyId: 'pty-1',
|
||||
worktreeId: 'wt1',
|
||||
tabId: 'tab-1',
|
||||
leafId: TEST_LEAF_1,
|
||||
state: 'attached'
|
||||
})
|
||||
store.upsertSshRemotePtyLease({
|
||||
targetId: 'target-1',
|
||||
ptyId: 'pty-2',
|
||||
worktreeId: 'wt1',
|
||||
tabId: 'tab-2',
|
||||
leafId: TEST_LEAF_2,
|
||||
state: 'attached'
|
||||
})
|
||||
store.retireLeaseAndReap('target-1', 'pty-1', 'pane closed')
|
||||
store.retireLeaseSparingPty('target-1', 'pty-2', 'relay pty missing')
|
||||
store.flush()
|
||||
|
||||
// Why: normalize is a strict allowlist, so set-then-read would pass even if the fields never persisted.
|
||||
const reloaded = await createStore()
|
||||
const reaped = reloaded.getSshRemotePtyLeases('target-1').find((l) => l.ptyId === 'pty-1')
|
||||
const spared = reloaded.getSshRemotePtyLeases('target-1').find((l) => l.ptyId === 'pty-2')
|
||||
expect(reaped?.state).toBe('terminated')
|
||||
expect(reaped?.retiredReason).toBe('pane closed')
|
||||
expect(reaped?.pendingRemoteShutdown).toBe(true)
|
||||
expect(spared?.state).toBe('expired')
|
||||
expect(spared?.retiredReason).toBe('relay pty missing')
|
||||
expect(spared?.pendingRemoteShutdown).toBeUndefined()
|
||||
})
|
||||
|
||||
it('ignores retirement of a lease that does not exist', async () => {
|
||||
const store = await createStore()
|
||||
store.upsertSshRemotePtyLease({
|
||||
targetId: 'target-1',
|
||||
ptyId: 'pty-1',
|
||||
state: 'attached'
|
||||
})
|
||||
const before = structuredClone(store.getSshRemotePtyLeases())
|
||||
|
||||
store.retireLeaseAndReap('target-1', 'pty-absent', 'pane closed')
|
||||
store.retireLeaseSparingPty('target-other', 'pty-1', 'relay generation replaced')
|
||||
|
||||
expect(store.getSshRemotePtyLeases()).toEqual(before)
|
||||
})
|
||||
|
||||
it('keeps worktree deletion authoritative against stale writes and later same-path reuse', async () => {
|
||||
const store = await createStore()
|
||||
store.setWorktreeMeta('wt1', { displayName: 'Worktree' })
|
||||
|
||||
+52
-1
@@ -1605,7 +1605,13 @@ function normalizeSshRemotePtyLease(value: unknown): SshRemotePtyLease | null {
|
||||
createdAt: typeof raw.createdAt === 'number' ? raw.createdAt : now,
|
||||
updatedAt: typeof raw.updatedAt === 'number' ? raw.updatedAt : now,
|
||||
...(typeof raw.lastAttachedAt === 'number' ? { lastAttachedAt: raw.lastAttachedAt } : {}),
|
||||
...(typeof raw.lastDetachedAt === 'number' ? { lastDetachedAt: raw.lastDetachedAt } : {})
|
||||
...(typeof raw.lastDetachedAt === 'number' ? { lastDetachedAt: raw.lastDetachedAt } : {}),
|
||||
// Why: this is a strict allowlist — an unlisted field is dropped on load, so a retirement flag would
|
||||
// work in every in-process test and vanish on the first restart, in exactly the durable case it exists for.
|
||||
...(typeof raw.retiredReason === 'string' && raw.retiredReason.length <= 512
|
||||
? { retiredReason: raw.retiredReason }
|
||||
: {}),
|
||||
...(raw.pendingRemoteShutdown === true ? { pendingRemoteShutdown: true } : {})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -7182,6 +7188,51 @@ export class Store {
|
||||
this.flush()
|
||||
}
|
||||
|
||||
// Why: D0 — the only two ways to retire a lease. The split is by what the evidence proves, not by which
|
||||
// handler produced it: I3 (the pane is provably gone) authorizes reaping the remote PTY; I6 (the row is
|
||||
// wrong about the PTY it names) never does.
|
||||
retireLeaseAndReap(targetId: string, ptyId: string, reason: string): void {
|
||||
this.retireSshRemotePtyLease(targetId, ptyId, 'terminated', reason, true)
|
||||
}
|
||||
|
||||
// `pendingRemoteShutdown` is unreachable from here by construction, and that is the point: a reset relay
|
||||
// reuses `pty-N`, so the id a wrong row names may already be another pane's live shell.
|
||||
retireLeaseSparingPty(targetId: string, ptyId: string, reason: string): void {
|
||||
this.retireSshRemotePtyLease(targetId, ptyId, 'expired', reason, false)
|
||||
}
|
||||
|
||||
private retireSshRemotePtyLease(
|
||||
targetId: string,
|
||||
ptyId: string,
|
||||
state: Extract<SshRemotePtyLease['state'], 'terminated' | 'expired'>,
|
||||
reason: string,
|
||||
pendingRemoteShutdown: boolean
|
||||
): void {
|
||||
const relayPtyId = this.getRelayPtyIdForSshLeaseStorage(targetId, ptyId)
|
||||
const lease = this.state.sshRemotePtyLeases?.find(
|
||||
(entry) => entry.targetId === targetId && entry.ptyId === relayPtyId
|
||||
)
|
||||
if (!lease) {
|
||||
return
|
||||
}
|
||||
// Why: retirement is terminal. Re-retiring would let a spared row later acquire a reap flag for a PTY
|
||||
// we already decided we have no claim on; keep binding cleanup idempotent as the singular setter does.
|
||||
if (lease.state === 'terminated' || lease.state === 'expired') {
|
||||
if (this.clearSshRemotePtyBindingsForLeases(targetId, [lease])) {
|
||||
this.flush()
|
||||
}
|
||||
return
|
||||
}
|
||||
lease.state = state
|
||||
lease.updatedAt = Date.now()
|
||||
lease.retiredReason = reason
|
||||
if (pendingRemoteShutdown) {
|
||||
lease.pendingRemoteShutdown = true
|
||||
}
|
||||
this.clearSshRemotePtyBindingsForLeases(targetId, [lease])
|
||||
this.flush()
|
||||
}
|
||||
|
||||
removeSshRemotePtyLease(targetId: string, ptyId: string): void {
|
||||
const relayPtyId = this.getRelayPtyIdForSshLeaseStorage(targetId, ptyId)
|
||||
const leases = (this.state.sshRemotePtyLeases ?? []).filter(
|
||||
|
||||
@@ -203,6 +203,14 @@ export type SshRemotePtyLease = {
|
||||
updatedAt: number
|
||||
lastAttachedAt?: number
|
||||
lastDetachedAt?: number
|
||||
/**
|
||||
* Present iff Orca retired this row itself; a relay-driven expiry leaves it absent. That distinction
|
||||
* is load-bearing, not triage colour: `'expired'` alone cannot authorize pane recovery, because we
|
||||
* write it too. Free-form so a retirement can carry its detail (expected vs managed identity).
|
||||
*/
|
||||
retiredReason?: string
|
||||
/** Set only by `retireLeaseAndReap` (I3): the pane is provably gone, so this relay PTY is ours to kill. */
|
||||
pendingRemoteShutdown?: boolean
|
||||
}
|
||||
|
||||
/** Main-owned relay lease needed to reclaim PTY delivery after a desktop restart. */
|
||||
|
||||
Reference in New Issue
Block a user