Skip to content

fix(relay): gate relay control work on proven broker liveness - #16021

Merged
nwparker merged 1 commit into
mainfrom
nwparker/relay-broker-liveness-gate
Aug 25, 2026
Merged

nwparker merged 1 commit into
mainfrom
nwparker/relay-broker-liveness-gate

Conversation

@nwparker

@nwparker nwparker commented Aug 23, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​148 $\color{#cf222e}{\Huge{\mathbf{−}}}$​6 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​142
Prod 2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​12 $\color{#cf222e}{\Huge{\mathbf{−}}}$​5 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​7

Description

After a relay control socket died, broker ownership could remain valid while the control itself was unusable. Pairing and endpoint work read that stale ownership handle and failed with relay_control_not_active instead of waiting for the reconcile already replacing it.

Control work now uses a liveness-gated broker accessor and wait path. The raw ownership accessor remains available only for revoke identity matching, where retaining the original owner is required for durable retry.

ELI5

Orca remembered which relay phone line belonged to it, but sometimes that line had already gone dead. Pairing grabbed the remembered-but-dead line while Orca was already connecting a replacement. Now pairing waits for the working replacement; revocation still remembers the old line long enough to finish its bookkeeping.

Reliability contract

  • A broker is withheld only when it proves isLive() === false.
  • Missing liveness evidence remains unverifiable, not exited, and stays usable.
  • A failed reconcile preserves the existing bounded retry/backoff behavior and the existing relay_control_not_active terminal error.
  • Revoke routing retains raw ownership identity and its durable outbox semantics.

Readiness review

  • SSH / remote / network: uses locally observed control state; loss of contact alone is never treated as process death. Reconnect and retry ownership are unchanged.
  • Crash / retry / growth: no new timers, loops, queues, casts, or unbounded collections; the wait path terminates on the existing reconcile generation.
  • Security / supply chain: no dependency, credential, shell, persistence, or network-sink changes.
  • Performance / resources: one constant-time liveness predicate on a demand path that already reconciles; no polling or subprocess changes.
  • Functional correctness: real coordinator + service coverage proves dead-broker replacement, live-broker reuse, and unverifiable-broker retention.
  • Backward compatibility / data loss: no persisted state, RPC, stream, mobile, or protocol surface changed.
  • Cross-platform / remoting: platform-neutral TypeScript; the original report was Windows, and no path/shell/process assumptions were added.

Validation

  • src/main/runtime/relay: 12 files / 84 tests passed
  • pnpm run typecheck:node: passed
  • pnpm run check:code-quality:changed: passed with 0 findings
  • GitHub required checks: passed, including Node 24/26 shards, Windows packaging, wire compatibility, static analysis, and typecheck

Review verdict

  • P0: none
  • P1: none
  • P2: none

Ready to land.

Pairing a phone would fail with relay_control_not_active after the desktop
had been asleep or off Wi-Fi, and stayed broken until the app was restarted —
retrying the QR code just reproduced the same error.

The auth coordinator keeps its broker ownership marked valid across a control
socket death; only reconcile applied a liveness gate before republishing
'registered'. Control work went through getActiveBroker/waitForActiveBroker,
which returned that still-"valid" ownership, so create_pairing_relay and
get_endpoints kept driving the dead control instead of letting the coordinator
open a replacement.

Control paths now use getLiveBroker/waitForLiveBroker, which apply the same
liveness gate reconcile uses, so a dead broker is withheld and the demand wait
resolves on its replacement. getActiveBroker stays the raw ownership handle for
revoke identity matching, which must still match the broker it queued against.

Liveness stays proof-based: only a broker that reports isLive() === false is
withheld. A broker that cannot report liveness is unverifiable, not exited, and
remains usable — silence never costs a working relay.
@coderabbitai

coderabbitai Bot commented Aug 23, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4cdd8dcb-ca40-428d-aa63-dbcdcda298a0

📥 Commits

Reviewing files that changed from the base of the PR and between 063b804 and e41b5a6.

📒 Files selected for processing (5)
  • src/main/runtime/relay/desktop-relay-service-broker-liveness.test.ts
  • src/main/runtime/relay/desktop-relay-service.ts
  • src/main/runtime/relay/relay-auth-coordinator-recovery.test.ts
  • src/main/runtime/relay/relay-auth-coordinator.test.ts
  • src/main/runtime/relay/relay-auth-coordinator.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The relay authentication coordinator now provides getLiveBroker() and waitForLiveBroker(). Dead brokers remain available for identity matching but are excluded from control work. Brokers without liveness reporting remain usable. DesktopRelayService now waits for live brokers during demand. Tests cover broker replacement, live broker reuse, dead broker filtering, and unverifiable broker handling.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description gives a detailed rationale, scope, reliability contract, testing results, and review verdict, but omits required template sections such as Linked Issue and the checklist. Add the required template sections, including a linked issue, explicit Visual Proof status, Testing checklist, AI Disclosure, Agent skill boundary, Notes, and final Checklist.
✅ Passed checks (4 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring check was indeterminate for this PR — some files could not be analyzed in time. Not blocking.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the main change: gating relay control work on broker liveness.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@nwparker
nwparker merged commit 7e79476 into main Aug 25, 2026
48 checks passed
paidaxingyo666 pushed a commit to paidaxingyo666/Manta that referenced this pull request Aug 25, 2026
enrick-dev pushed a commit to enrick-dev/orca that referenced this pull request Oct 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant