mirror of
https://github.com/stablyai/orca.git
synced 2026-10-23 16:03:49 +00:00
fix(ssh): deliver a remote pane's exit that no operation could attribute
classifyPendingExit stamped every same-id exit as classified before deciding its verdict, including the 'unverifiable' returns where the operation said it could not attribute the exit at all. On relays that send no incarnation ids, an operation that bound to the PTY after the exit was recorded could stamp the quarantine it does not hold; the real holder's finish() then refused to release it and zero pty:exit reached the renderer for a PTY the host reported dead. Claim an exit only when the operation actually attributed it: it owned the PTY when the exit arrived, or both incarnations are known and comparable. recordExit also returned false for an unheld exit and let the notification router publish it directly, bypassing the quarantine's published latch, so a late binder that later attributed the same exit published a second pane death. Publish it through the latch instead and report that the tracker owns it.
This commit is contained in:
@@ -30,3 +30,72 @@ it('does not release a quarantined exit when one operation finishes twice', () =
|
||||
|
||||
expect(publish).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('releases a held exit when a late binder without an incarnation cannot attribute it', () => {
|
||||
const tracker = new SshPtySpawnExitRaceTracker()
|
||||
const publish = vi.fn()
|
||||
const holder = tracker.begin('pty-1')
|
||||
const late = tracker.begin()
|
||||
|
||||
// Relays that send no incarnation id leave both sides unidentified, so nothing is attributable.
|
||||
tracker.recordExit('pty-1', undefined, publish)
|
||||
tracker.bind(late, 'pty-1')
|
||||
expect(tracker.classifyPendingExit(late, { id: 'pty-1' })).toBe('unverifiable')
|
||||
tracker.finish(late)
|
||||
tracker.finish(holder)
|
||||
|
||||
expect(publish).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('releases a held exit when a late binder attaches without an incarnation', () => {
|
||||
const tracker = new SshPtySpawnExitRaceTracker()
|
||||
const publish = vi.fn()
|
||||
const holder = tracker.begin('pty-1')
|
||||
const late = tracker.begin()
|
||||
|
||||
tracker.recordExit('pty-1', 'incarnation-old', publish)
|
||||
tracker.bind(late, 'pty-1')
|
||||
expect(tracker.classifyPendingExit(late, { id: 'pty-1' })).toBe('unverifiable')
|
||||
tracker.finish(late)
|
||||
tracker.finish(holder)
|
||||
|
||||
expect(publish).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('publishes an unheld exit once even after a late binder attributes it', () => {
|
||||
const tracker = new SshPtySpawnExitRaceTracker()
|
||||
const publish = vi.fn()
|
||||
const late = tracker.begin()
|
||||
|
||||
// The notification router publishes whatever the tracker does not take ownership of.
|
||||
if (!tracker.recordExit('pty-1', 'incarnation-1', publish)) {
|
||||
publish()
|
||||
}
|
||||
tracker.bind(late, 'pty-1')
|
||||
expect(tracker.classifyPendingExit(late, { id: 'pty-1', incarnationId: 'incarnation-1' })).toBe(
|
||||
'exited'
|
||||
)
|
||||
tracker.finish(late)
|
||||
|
||||
expect(publish).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('cannot recall an exit released before a late binder attributes it', () => {
|
||||
const tracker = new SshPtySpawnExitRaceTracker()
|
||||
const publish = vi.fn()
|
||||
const holder = tracker.begin('pty-1')
|
||||
const late = tracker.begin()
|
||||
|
||||
tracker.recordExit('pty-1', 'incarnation-1', publish)
|
||||
// holders reaches zero before the late binder reaches its verdict, so the exit escapes.
|
||||
tracker.finish(holder)
|
||||
expect(publish).toHaveBeenCalledOnce()
|
||||
|
||||
tracker.bind(late, 'pty-1')
|
||||
expect(tracker.classifyPendingExit(late, { id: 'pty-1', incarnationId: 'incarnation-1' })).toBe(
|
||||
'exited'
|
||||
)
|
||||
tracker.finish(late)
|
||||
|
||||
expect(publish).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
@@ -32,6 +32,7 @@ export class SshPtySpawnExitRaceTracker {
|
||||
operation.relayPtyId = relayPtyId
|
||||
}
|
||||
|
||||
/** True when the tracker owns publication of this exit: deferred to a holder, or already sent. */
|
||||
recordExit(relayPtyId: string, incarnationId: unknown, publish?: () => void): boolean {
|
||||
let published = false
|
||||
const quarantine: QuarantinedSshPtyExit = {
|
||||
@@ -60,7 +61,15 @@ export class SshPtySpawnExitRaceTracker {
|
||||
...(tracked ? { held, quarantine } : {})
|
||||
})
|
||||
}
|
||||
return quarantine.holders > 0
|
||||
if (publish === undefined) {
|
||||
return false
|
||||
}
|
||||
if (quarantine.holders === 0) {
|
||||
// Why: no operation can fence this exit, so send it now — through the quarantine, so a
|
||||
// later binder that attributes it cannot deliver the same death a second time.
|
||||
quarantine.publish()
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
classifyPendingExit(
|
||||
@@ -69,8 +78,14 @@ export class SshPtySpawnExitRaceTracker {
|
||||
): SshPtyPendingExitOutcome {
|
||||
const sameIdExits = operation.exits.filter((exit) => exit.relayPtyId === result.id)
|
||||
for (const exit of sameIdExits) {
|
||||
// Why: a reached verdict owns the exit, so finish() must not second-guess it and re-publish.
|
||||
if (exit.quarantine) {
|
||||
// Why: classifying claims the exit and silences its holder's release, so only claim what this
|
||||
// operation actually attributed — it owned the PTY when the exit arrived, or both incarnations
|
||||
// are known and comparable. A late binder that cannot compare identities has proven nothing,
|
||||
// and stamping its "I can't tell" would strand a death the host reported.
|
||||
const attributed =
|
||||
exit.held === true ||
|
||||
(result.incarnationId !== undefined && exit.incarnationId !== undefined)
|
||||
if (exit.quarantine && attributed) {
|
||||
exit.quarantine.classified = true
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user