fix(pair-relay): keep pairing socket open past desktop timeout

[santhiprakash]

Summary

buzz-pair-relay closed every pairing WebSocket at 120s. Buzz Desktop's pairing hard timeout is 130s and is the path that emits Session timed out / the expired QR state. The sidecar always won by 10s, so leaving the QR on screen produced the transport string relay connection closed instead.

Related issue

Fixes #6033

Testing

• cargo test -p buzz-pair-relay → 1 lib + 51 integration tests passed
• cargo clippy -p buzz-pair-relay --all-targets -- -D warnings → passed
• cargo fmt -p buzz-pair-relay → applied

New coverage: CONN_TIMEOUT must exceed the desktop 130s deadline; virtual-time close tests now advance CONN_TIMEOUT + 1s instead of a hardcoded 121s.

[themiguelamador]

I found three issues in the timeout change:\n\n1. The new invariant test hard-coded Desktop's 130-second value inside the pair-relay crate. That test would stay green if Desktop changed, so it did not actually protect the cross-crate contract.\n2. The two deadlines did not begin at the same point. The relay starts its 140-second lifetime when the socket is accepted, while Desktop started 130 seconds only after NIP-42 handling and the EOSE wait. That setup can consume up to 13 seconds, allowing the relay to close first despite the nominal 10-second margin.\n3. Deployment guidance required proxy read timeouts but gave no lower bound. A shorter ingress timeout could still preempt both application deadlines.\n\nI fixed these on Complear/buzz branch (commit ): Desktop now starts its named hard timeout immediately after the WebSocket split, a Desktop test compares the actual public constants across crates, the false hard-coded relay test is removed, and the relay/chart docs require a proxy idle/read timeout of at least 140 seconds.\n\nValidation:\n-
running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

running 51 tests
test test_backpressure_unit ... ok
test test_event_shape_missing_id ... ok
test test_event_tag_string_too_long ... ok
test test_binary_frame ... ok
test test_conn_timeout ... FAILED
test test_cancellation_immediate ... FAILED
test test_close_keeps_connection ... ok
test test_close_handshake ... ok
test test_event_missing_sig ... ok
test test_event_too_many_tags ... ok
test test_graceful_close ... ok
test test_invalid_hex_in_event_id ... ok
test test_invalid_hex_in_p_filter ... ok
test test_json_injection_sub_id ... ok
test test_kind_rejection ... ok
test test_malformed_req_non_object_filter ... ok
test test_malformed_req_non_string_sub_id ... ok
test test_malformed_req_too_few ... ok
test test_max_frame_size ... ok
test test_multi_value_p ... ok
test test_multiple_filters ... ok
test test_multiple_p_tags ... ok
test test_negative_created_at ... ok
test test_no_p_filter ... ok
test test_no_p_tag ... ok
test test_overlong_sub_id ... ok
test test_multiple_subscribers_same_p ... ok
test test_ping_pong ... ok
test test_fan_out_drop_doesnt_close ... ok
test test_control_msg_backpressure ... ok
test test_second_sub_different_id ... ok
test test_req_after_close ... ok
test test_second_sub_same_id ... ok
test test_unknown_message ... ok
test test_unsupported_filter_field ... ok
test test_event_rate_limit ... ok
test test_uppercase_hex_in_event_fields ... ok
test test_uppercase_hex_in_p_filter ... ok
test test_reader_backpressure_closes ... ok
test test_write_timeout ... ok
test test_message_rate_limit ... ok
test test_ping_counts_toward_rate_limit ... ok
test test_no_client_data_in_logs ... ok
test test_global_conn_cap ... ok
test test_conn_counter_no_leak ... ok
test test_close_unknown_sub_id ... ok
test test_eose_try_send_failure ... ok
test test_live_delivery ... ok
test test_no_replay ... ok
test test_close_removes_sub ... ok
test test_no_events_after_close ... ok

failures:

---- test_conn_timeout stdout ----
buzz-pair-relay listening on 127.0.0.1:57516
conn opened conn_id=0 active=1
conn closed conn_id=0 active=0

thread 'test_conn_timeout' (10693676) panicked at crates/buzz-pair-relay/tests/integration.rs:81:19:
connection did not close within 2 s
note: run with RUST_BACKTRACE=1 environment variable to display a backtrace

---- test_cancellation_immediate stdout ----
buzz-pair-relay listening on 127.0.0.1:57519
conn opened conn_id=0 active=1
conn closed conn_id=0 active=0

thread 'test_cancellation_immediate' (10693670) panicked at crates/buzz-pair-relay/tests/integration.rs:81:19:
connection did not close within 2 s

failures:
test_cancellation_immediate
test_conn_timeout

test result: FAILED. 49 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.60s: 51 passed\n- Desktop Rust library suite: 2,441 passed, 15 ignored\n- Locked cross-crate contract test: passed\n- Strict all-target Clippy for pair relay and the Desktop workspace: passed\n- Rust formatting and : passed

[themiguelamador]

I found three issues in the timeout change:

1. The new invariant test hard-coded Desktop's 130-second value inside the pair-relay crate. That test would stay green if Desktop changed, so it did not actually protect the cross-crate contract.
2. The two deadlines did not begin at the same point. The relay starts its 140-second lifetime when the socket is accepted, while Desktop started 130 seconds only after NIP-42 handling and the EOSE wait. That setup can consume up to 13 seconds, allowing the relay to close first despite the nominal 10-second margin.
3. Deployment guidance required proxy read timeouts but gave no lower bound. A shorter ingress timeout could still preempt both application deadlines.

I fixed these on Complear/buzz branch review/pr-6057-fix (commit 43b6a81b1): Desktop now starts its named hard timeout immediately after the WebSocket split, a Desktop test compares the actual constants across crates, the false hard-coded relay test is removed, and the relay/chart docs require a proxy idle/read timeout of at least 140 seconds.

Validation:

• cargo test -p buzz-pair-relay: 51 passed
• Desktop Rust library suite: 2,441 passed, 15 ignored
• Locked cross-crate contract test: passed
• Strict all-target Clippy for pair relay and the Desktop workspace: passed
• Rust formatting and git diff --check: passed

[santhiprakash]

Addressed in e122402:

1. Hard-coded 130s in the pair-relay test — removed that crate-local invariant. Desktop now exports PAIRING_HARD_TIMEOUT, and pairing_timeout_contract_tests compares it to buzz_pair_relay::CONN_TIMEOUT, so a desktop timeout change fails the contract test.
2. Deadlines starting at different points — PAIRING_HARD_TIMEOUT now starts immediately after the WebSocket split, before NIP-42/EOSE. Setup no longer consumes the desktop budget while the sidecar's 140s clock is already running.
3. Proxy timeout lower bound — pair-relay crate docs and deploy/charts/buzz/README.md now require idle/read timeouts of at least 140 seconds.

Regression check:

• cargo test -p buzz-pair-relay --test integration -- test_conn_timeout test_cancellation_immediate → 2 passed
• cargo test --manifest-path desktop/src-tauri/Cargo.toml --lib pairing → 15 passed
• cargo clippy -p buzz-pair-relay --all-targets -- -D warnings → passed
• cargo clippy --manifest-path desktop/src-tauri/Cargo.toml --all-targets -- -D warnings → passed

Status: ready for re-review at e12240254.

[themiguelamador]

The follow-up resolves all three findings from my earlier review: Desktop starts its hard timeout at the transport boundary, the regression test compares the real Desktop and pair-relay constants, and deployment docs specify the 140-second proxy minimum.

Re-verification on the updated head:

• cargo test -p buzz-pair-relay --quiet — 51 passed
• locked Desktop cross-crate timeout contract — 1 passed
• workspace and Desktop Tauri Rust formatting checks
• git diff --check

[ravarora2]

🤖 Independent re-review of commit 3ef6067efb48baa2462579466ac9be7f75e42c6e.

This commit addresses the two blockers from a separate review:

1. It protects the relay before a WebSocket is established. Hyper now enforces a 10-second HTTP header-read timeout. A client that opens a TCP connection and sends an incomplete upgrade request is closed promptly and does not consume one of the relay's 128 upgraded-WebSocket slots. The deployment guidance now clearly separates:

   • short pre-upgrade HTTP header/request timeouts and connection/rate limits; and
   • post-upgrade WebSocket read/send/idle timeouts longer than the relay's 140-second lifetime.

2. It causally protects the Desktop timeout ordering. The Desktop's 130-second pairing timer is created immediately after the WebSocket is established, before NIP-42 authentication and EOSE subscription setup. The new paused-time test spends 11 seconds in that setup path and proves Desktop still emits Session timed out at 130 seconds from connection establishment, before the relay's simulated 140-second close.

The commit also stabilizes two existing virtual-time relay timeout tests by waiting for server cleanup before asserting client closure.

I independently reviewed the production paths and ran both new focused tests at this exact head. I also inspected the full-suite, live-probe, and mutation receipts: removing the Hyper timeout breaks the incomplete-header regression, and moving the Desktop timer below setup breaks the 130-second regression. No new code blocker was found.

Residual non-blocker: the sidecar still does not impose a separate global cap on raw pre-upgrade TCP connections, so operator-side ingress connection/rate limits remain required; the revised documentation states that requirement. GitHub's remaining Desktop/Windows jobs are still running, so final merge readiness is conditional on those checks finishing green.

[ravarora2]

lgtm! Waiting for the owners for the final review

[santhiprakash]

Thanks for the thorough re-review — glad the header-read timeout and WebSocket slot protection changes address the blockers.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Reviewed the timeout contract at exact head b510461c9d98b96c308504675ee81ba78bcb386a. I found no actionable blocker.

The relay now outlives Desktop's pairing deadline (140s vs. 130s), Desktop creates its timer at the live transport boundary before the bounded NIP-42/EOSE setup, and the paused-time regression exercises the production task through 11s of setup before proving the Desktop-owned Session timed out result at 130s. The proxy guidance also correctly distinguishes short pre-upgrade limits from post-upgrade WebSocket limits greater than 140s, while Hyper independently bounds incomplete headers at 10s.

Focused verification at this exact head:

• pair-relay timeout, cancellation, and incomplete-header regressions: 3 passed
• git diff --check: passed
• Desktop locked test compilation was attempted but this machine lacks CMake for the existing vendored Opus build; I therefore relied on source-level test review rather than claiming a local Desktop pass

The test-only pair-relay dependency adds some lockfile weight, but I do not consider that a correctness blocker.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Correction to my earlier clear review: requesting changes after a late concurrency finding exposed an uncovered blocking path.

The new 130-second timer is created before NIP-42/EOSE, but it is not polled during those setup awaits or while a socket write is in progress. In particular, the subscription write.send(...).await occurs before the select!, and the outbound and recovery branches await further write.send calls inside a selected branch. If the peer stops reading after the WebSocket handshake, a send can remain pending past 130 seconds; Desktop then fails to emit its promised Session timed out state before the relay's 140-second cleanup.

This was reproduced at exact head b510461c9d98b96c308504675ee81ba78bcb386a with a one-byte duplex WebSocket peer that never reads: after paused time advanced through PAIRING_HARD_TIMEOUT, the production pairing task was still running. Please enforce the hard deadline and cancellation across setup and every direct socket write (or wrap the complete post-connect task in an equivalent deadline), and add the stalled-write regression.

Relay accounting remains sound; this finding is confined to Desktop deadline enforcement.

[santhiprakash]

Addressed in a07e3bc — the post-connect pairing work now runs in a spawned worker task wrapped by an outer tokio::select! that fires at PAIRING_HARD_TIMEOUT and aborts the worker.

Why not timeout() alone: timeout() only drops the future in-place on the same task; it cannot cancel an in-flight SinkExt::send await, so a stalled write blocked the old loop and the pinned sleep never polled.

Coverage: NIP-42 auth, the pre-loop subscription write.send, and outbound/recovery writes all live inside pairing_ws_session_loop; the outer deadline aborts the whole worker regardless of which await is blocked.

Regression: stalled_subscribe_write_emits_session_timeout — 1-byte duplex peer that never reads, virtual time advanced past PAIRING_HARD_TIMEOUT, asserts the task finishes and pairing-error emits "Session timed out".

Validation (a07e3bc):

• cargo test pairing_timeout_contract — 3 passed (~0.01s)
• cargo test -p buzz-pair-relay — 52 passed
• cargo fmt; git diff --check clean

Re-requesting review.