fix(relay): make the quit teardown terminal on the committed handler, not the vetoable one

Moving quit off the re-armable fence was right; putting the terminal stop() in
`before-quit` was not. That handler is vetoable — a renderer beforeunload can cancel
the quit — and nothing anywhere clears `stopped`. `refreshDemand` returns early on it,
so a quit the user backed out of left Relay dead for the rest of the session, with no
liveness tick, no reconcile and no door back. The comment three lines below already
warns about exactly this, and `desktopPushService` in the same file is split across the
two handlers for exactly this reason.

`before-quit` now fences: it closes the broker just as promptly and latches `fenced`,
which `refreshDemand` also refuses, so the settling mint, the invite-expiry wake and the
power-resume `ensureLive` still cannot reopen one in the window before `will-quit`. On a
veto the next auth mutation clears `fenced` and Relay comes back. `will-quit` — the
committed path — calls stop() and makes it terminal. The original goal is intact; only
the irrecoverable case is gone.

The prior justification ("the provider null below already ends pairing even on a vetoed
quit") does not hold up: that is a second defect on the vetoed-quit path, not a licence
to add a third.

The call-site ratchet guarded the file, which could not see this — the quit module held
both a latch and a veto. It now resolves each handler body and guards them separately:
the vetoable one must fence and must not latch, the committed one the reverse.
Mutation-verified — restoring the `before-quit` stop() fails both new cases.
This commit is contained in:
Neil
2026-09-16 23:41:07 -07:00
parent c0907f78c8
commit 77d8b28aa2
2 changed files with 44 additions and 13 deletions
@@ -11,9 +11,14 @@ import { describe, expect, it } from 'vitest'
* fence, and a settling mint, a pending invite expiry or a power-resume `ensureLive` each reopened
* a broker after quit; the damage only shows up in timer state, minutes later.
*
* This test is the compile error that swap cannot produce. Both directions matter: quit must not
* regain the re-armable fence, and sign-out/relaunch must not lose it — collapsing them either way
* destroys the distinction the defect came from.
* The quit *module* is not the unit — the handler is. `before-quit` is vetoable (a renderer
* beforeunload can cancel it) and nothing clears `stopped`, so a terminal stop() there kills Relay
* for the rest of the session on a quit the user backed out of. `will-quit` is the committed path.
* The file already splits `desktopPushService` exactly this way, for exactly this reason.
*
* This test is the compile error those swaps cannot produce. Every direction matters: the vetoable
* handler must not latch, the committed handler must, and sign-out/relaunch must keep the fence —
* collapsing any pair destroys the distinction the defect came from.
*
* If you are here because this went red on a refactor that looks harmless, that is the intended
* trade: a ratchet that fails loudly on a benign change beats one that passes silently on a
@@ -34,12 +39,31 @@ function startupSource(module: string): string {
return readFileSync(join(STARTUP_DIR, module), 'utf8')
}
describe('desktop relay teardown call sites', () => {
it('ends the relay terminally at quit, never through the re-armable fence', () => {
const source = startupSource(QUIT_MODULE)
/** Body of one `function install<Name>Handler()`, up to the next top-level `function`. */
function handlerBody(source: string, installer: string): string {
const start = source.indexOf(`function ${installer}(`)
expect(start, `${installer} not found — the ratchet cannot see what it guards`).toBeGreaterThan(
-1
)
const rest = source.slice(start)
const end = rest.indexOf('\nfunction ', 1)
return end === -1 ? rest : rest.slice(0, end)
}
expect(source).toMatch(RELAY_TERMINAL_STOP)
expect(source).not.toMatch(RELAY_REARMABLE_FENCE)
describe('desktop relay teardown call sites', () => {
it('fences the relay on the vetoable quit handler, and never latches it there', () => {
const beforeQuit = handlerBody(startupSource(QUIT_MODULE), 'installBeforeQuitHandler')
expect(beforeQuit).toMatch(RELAY_REARMABLE_FENCE)
// The latch has nothing that clears it, so a vetoed quit would end Relay for the session.
expect(beforeQuit).not.toMatch(RELAY_TERMINAL_STOP)
})
it('ends the relay terminally on the committed quit handler', () => {
const willQuit = handlerBody(startupSource(QUIT_MODULE), 'installWillQuitHandler')
expect(willQuit).toMatch(RELAY_TERMINAL_STOP)
expect(willQuit).not.toMatch(RELAY_REARMABLE_FENCE)
})
it('keeps sign-out and relaunch on the fence, so the next auth mutation re-arms', () => {
+12 -5
View File
@@ -71,11 +71,14 @@ function installBeforeQuitHandler(): void {
})
}
state.isQuitting = true
// Why stop() and not the fence: the fence is re-armable by design (sign-out
// waits for the next auth mutation), so at quit a settling mint, an invite
// expiry or a power-resume could still reopen a broker. Quit is terminal,
// and the provider null below already ends pairing even on a vetoed quit.
state.desktopRelayService?.stop()
// Why the fence here and the terminal stop() in will-quit: this handler is vetoable, and
// stop() latches `stopped` with nothing to clear it — a renderer beforeunload that cancelled
// the quit would leave the relay dead for the rest of the session. The fence closes the broker
// just as promptly and latches `fenced`, which refreshDemand also refuses, so a settling mint,
// an invite expiry or a power-resume cannot reopen one in the window before will-quit either.
// On a veto the next auth mutation clears `fenced` and Relay comes back; on the committed path
// will-quit makes it terminal. Same reasoning as desktopPushService, one handler down.
state.desktopRelayService?.fenceAndCloseNow()
state.runtimeRpc?.setMobileRelayPairingProvider(null)
state.unsubscribeAgentAwakeStatusChanges?.()
state.unsubscribeAgentAwakeStatusChanges = null
@@ -111,6 +114,10 @@ function installWillQuitHandler(): void {
}
// A renderer can veto before-quit; push must survive until quit is committed.
state.desktopPushService?.stop()
// Same reason, and the one the fence in before-quit defers to: stop() is terminal and nothing
// clears `stopped`, so it only belongs on the committed path. before-quit already fenced the
// broker closed; this is what stops it coming back.
state.desktopRelayService?.stop()
state.unsubscribeSystemResumeBroadcast?.()
state.unsubscribeSystemResumeBroadcast = null
// Why: renderer guards can still cancel before this committed phase; `log stream` must survive those vetoes.