fix(operator-stream): recover the single-operator slot from a dead peer

[martinburian-ba]

Summary

QuicOperatorServer::onConnected() unconditionally overwrote operatorConnection_, relying entirely on ba-quic-lib's maxConnections to keep out a second operator. That's fail-open when two handshakes reach CONNECTED simultaneously, and gave no way to reclaim the slot from a peer that vanished without a clean QUIC close (kill -9, crash, power loss) — a hard-killed GUI held streaming-control for the whole car until the ES process restarted.

Fix

Ports the fix already shipped in teleop-module's QuicOperatorServer (same class, same bug, fixed there first):

• onConnected() now does an explicit check-and-set under operatorMutex_, logging a warning and calling quicServer_->disconnect(id) for a second connection instead of silently replacing the tracked one.
• buildSettings() sets disconnectTimeoutMs explicitly (matches ba-quic-lib's own default; set explicitly so the choice reads as deliberate, per the teleop-module precedent).
• Stale doc comments claiming compare-and-swap logic wasn't needed are left in place (house rule: never rewrite existing comments) with a follow-up note appended explaining the correction.

Test plan

• No test infrastructure exists in this repo yet (confirmed — no test/ dir, no CTest/GTest wiring), so this wasn't covered by an automated test. Reviewed by hand against ba-quic-lib's vendored headers and against teleop-module's shipped fix for API/locking correctness.
• Repro from the ticket: connect a GUI with streaming-control enabled → kill -9 it → connect another GUI → commands should no longer be dropped.

BAF-1900

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: fb9f5994-b2b1-4d96-8e46-5a93886f5019

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[danprudky]

[CRITICAL] YouTrack ticket ID in comment: cpp-conventions forbids referencing YouTrack issues/task IDs in comments. This Doxygen doc comment leads with BAF-1900 update:. Describe the reasoning in plain language instead — the ticket ID belongs in the commit/PR, not the code.

[danprudky]

[CRITICAL] YouTrack ticket ID in comment: Same issue — the constructor comment leads with BAF-1900 update:. Reword without the ticket ID.

[danprudky]

[CRITICAL] YouTrack ticket ID in comment: Same issue — // BAF-1900: also covers a connection rejected.... Reword without the ticket ID.

[danprudky]

[IMPORTANT] Does this fix the reported dead-peer lockout, or only a different race?

BAF-1900's core symptom is a hard-killed GUI holding the operator slot for 30+ minutes while every new connection is rejected, and its suggested fix direction is "sane transport timeouts so a dead peer is detected, and/or newest-wins slot takeover."

This PR appears to do neither:

• idleTimeoutMs=30000 was already set before this diff, and per the PR description disconnectTimeoutMs=5000 merely restates ba-quic-lib's existing default — dead-peer detection timing doesn't actually change.
• The old onConnected() unconditionally overwrote operatorConnection_ on every accepted connection — whatever its other problems, that meant a new operator connecting after a stale peer's slot went dead would silently take over (de-facto newest-wins). This PR replaces that with an explicit check-and-reject: if operatorConnection_ still holds a value, the new connection is disconnected with "an operator is already connected, rejecting new one" — which is exactly the log line BAF-1900 quotes as the bug symptom.

If ba-quic-lib's maxConnections=1 really does let a second CONNECTED event through for a connection whose peer died without a clean close (which the ticket's field observation implies happens in practice, not just for simultaneous handshakes), this change makes that lockout deterministic rather than fixing it — it only handles the narrower simultaneous-handshake race the new doc comments describe.

Was this validated against the ticket's actual repro (kill -9 the GUI, then reconnect a second GUI), not just the simultaneous-CONNECTED race? If the repro still fails, this PR fixes a real but different bug, and BAF-1900 needs separate work (actual timeout tuning and/or an explicit newest-wins takeover instead of reject-on-occupied).

---

Suggested BAF-1900 comment draft, if the deviation is intentional or the repro still fails (for the PR author to post, not something I'll post myself):

This PR (#13) fixes a related but distinct race — two QUIC handshakes reaching CONNECTED simultaneously, which ba-quic-lib's own docs call out as fail-open even with maxConnections=1. It adds an explicit check-and-set in onConnected() to reject a second connection in that case.

It does not change idleTimeoutMs/disconnectTimeoutMs from their existing effective values, so it likely does not address the original repro (kill -9 a connected GUI, then reconnect a new one) — the new GUI will still be rejected by the same check-and-set logic while the dead peer's connection hasn't yet timed out at the transport layer. Recommend keeping this ticket open and tracking actual dead-peer timeout tuning (and/or newest-wins takeover) as follow-up work, either here or in a new ticket.