mirror of
https://github.com/stablyai/orca.git
synced 2026-09-28 16:02:45 +00:00
fix(relay): clear the demand-expiry timeout on a fence, not only in stop()
Why the existing lifecycle test missed this: it stubs
`demandLedger: { nextPendingExpiry: () => null }`, so the second timer this class
owns is never armed and the fence is only ever asserted against the liveness
interval. Change that one stub to a future expiry and the test fails.
`fenceAndCloseNow` cleared `livenessTimer` and left `demandExpiryTimer` armed.
`stop()` cleared both — but `stop()` has no production caller, while all three
fence sites (quit, sign-out, SIGNED_OUT) go through the path that cleared only one.
The consequence is the sharp part. The orphaned timeout fires `refreshDemand`,
which reconciles *and* re-installs the liveness interval the fence just tore down.
A fence during an outstanding pairing invite therefore undid itself minutes later
— the post-fence resurrection the comment two lines above forbids in so many words.
Both teardown paths now share one `disarmTimers()`. That also resolves the
`max-lines` pressure the extra clause created: the two teardown blocks were byte
identical, so deduplicating them is the right fix rather than a `max-lines`
disable, which this repo forbids and which the next person under that cap will be
tempted by.
The new test asserts `vi.getTimerCount() === 0` after the fence rather than
inferring from an absence of errors, and pins that a fence is not a stop — the
next auth mutation still re-arms both timers.
Pre-existing, not introduced by this stack; verified identical at the base SHA.
Mutation-tested: removing the `demandExpiryTimer` clause from `disarmTimers`
fails the new test at the timer-count assertion.
Also carries a note at `waitForLiveBrokerResult`: the `pending !== latestReconcile`
arm re-enters the loop without re-checking the deadline, so under continuous
reconcile churn the documented 20s budget is not strictly enforced and the
caller's transient demand ref is held past it. Not a hang — every iteration awaits
a settling promise — so it is bounded by how long churn lasts.
This commit is contained in:
@@ -193,3 +193,48 @@ describe('local-only mobile pairing', () => {
|
||||
).rejects.toThrow('relay_disabled_for_device')
|
||||
})
|
||||
})
|
||||
|
||||
describe('fence clears the pending demand-expiry timeout', () => {
|
||||
afterEach(() => vi.useRealTimers())
|
||||
|
||||
// The lifecycle test above stubs `nextPendingExpiry` to null, so it never arms the second timer
|
||||
// this class owns. With a pairing invite outstanding the fence left that timeout armed, and
|
||||
// `stop()` — the only method that cleared it — has no production caller. The orphaned timeout
|
||||
// then ran `refreshDemand`, which reconciles AND re-installs the liveness interval the fence had
|
||||
// just torn down: the post-fence resurrection the fence comment forbids.
|
||||
it('leaves no armed timer and cannot reconcile after a fence', () => {
|
||||
vi.useFakeTimers()
|
||||
const coordinator = {
|
||||
reconcile: vi.fn(),
|
||||
ensureLive: vi.fn(),
|
||||
fenceAndCloseNow: vi.fn(),
|
||||
stop: vi.fn()
|
||||
}
|
||||
const service = Object.create(DesktopRelayService.prototype) as DesktopRelayService
|
||||
Object.assign(service, {
|
||||
coordinator,
|
||||
// A pending invite: the QR is minted and its server-side expiry is still in the future.
|
||||
demandLedger: { nextPendingExpiry: () => Date.now() + 30_000 },
|
||||
stopped: false,
|
||||
livenessTimer: null,
|
||||
demandExpiryTimer: null
|
||||
})
|
||||
|
||||
service.start()
|
||||
expect(coordinator.reconcile).toHaveBeenCalledTimes(1)
|
||||
expect(vi.getTimerCount()).toBe(2)
|
||||
|
||||
service.fenceAndCloseNow()
|
||||
expect(vi.getTimerCount()).toBe(0)
|
||||
|
||||
vi.advanceTimersByTime(30 * 60_000)
|
||||
expect(coordinator.reconcile).toHaveBeenCalledTimes(1)
|
||||
expect(coordinator.ensureLive).not.toHaveBeenCalled()
|
||||
|
||||
// A fence is not a stop: the next auth mutation still re-arms both timers.
|
||||
service.authMutated()
|
||||
expect(coordinator.reconcile).toHaveBeenCalledTimes(2)
|
||||
expect(vi.getTimerCount()).toBe(2)
|
||||
service.stop()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -111,14 +111,24 @@ export class DesktopRelayService {
|
||||
this.refreshDemand()
|
||||
}
|
||||
|
||||
fenceAndCloseNow(hostCloseReason?: RelayHostCloseReason): void {
|
||||
// Why: a fence must be hard — a surviving liveness tick could catch the
|
||||
// window between the pre-sign-out fence and the profile wipe and briefly
|
||||
// resurrect a broker. The next auth mutation re-arms via refreshDemand.
|
||||
/** Disarms both timers this class owns. Every teardown needs both: the expiry timeout fires
|
||||
* `refreshDemand`, which reconciles and re-installs the liveness interval. */
|
||||
private disarmTimers(): void {
|
||||
if (this.demandExpiryTimer) {
|
||||
clearTimeout(this.demandExpiryTimer)
|
||||
this.demandExpiryTimer = null
|
||||
}
|
||||
if (this.livenessTimer) {
|
||||
clearInterval(this.livenessTimer)
|
||||
this.livenessTimer = null
|
||||
}
|
||||
}
|
||||
|
||||
fenceAndCloseNow(hostCloseReason?: RelayHostCloseReason): void {
|
||||
// Why: a fence must be hard — a surviving tick could catch the window between
|
||||
// the pre-sign-out fence and the profile wipe and briefly resurrect a broker.
|
||||
// The next auth mutation re-arms via refreshDemand.
|
||||
this.disarmTimers()
|
||||
this.coordinator.fenceAndCloseNow(hostCloseReason)
|
||||
}
|
||||
|
||||
@@ -222,14 +232,7 @@ export class DesktopRelayService {
|
||||
|
||||
stop(): void {
|
||||
this.stopped = true
|
||||
if (this.demandExpiryTimer) {
|
||||
clearTimeout(this.demandExpiryTimer)
|
||||
this.demandExpiryTimer = null
|
||||
}
|
||||
if (this.livenessTimer) {
|
||||
clearInterval(this.livenessTimer)
|
||||
this.livenessTimer = null
|
||||
}
|
||||
this.disarmTimers()
|
||||
this.coordinator.stop()
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user