fix(broker): tolerate slow Relaycast startup handshakes

Merged
KH
2mo ago
+464-17

Summary

  • raise the default Relaycast startup handshake from four 5-second attempts to three 12-second attempts
  • cap the complete broker handshake at 40 seconds so membership scaling and oversized overrides remain inside the SDK deadline with startup preparation time reserved
  • describe deadline exhaustion as an unconfirmed response that may have completed server-side, instead of declaring the backend unreachable
  • add regression coverage for measured slow success, multi-membership scaling, post-announcement setup reserve, and never-response bounds

Budget choice

Production returned successful POST /v1/workspaces responses in 7.4s and 9.5s. A 5s per-attempt deadline necessarily abandons both. The 12s default accepts the measured 9.5s success with 2.5s of headroom.

I verified the actual outer budget in the harness SDK rather than relying on the issue summary: packages/harness-driver/src/spawn-config.ts defaults startupTimeoutMs to 45,000ms, and packages/harness-driver/src/client.ts starts that polling deadline when the broker announces its bound API port. For one membership, three 12s attempts plus 250ms and 500ms retry backoffs exhaust in at most 36.75s. For multiple memberships or larger environment overrides, each attempt is shortened to the remaining portion of a 40s broker aggregate budget. This leaves five seconds for connection-file and startup-listener setup after port announcement and for the broker error to surface inside the SDK deadline. The SDK budget itself is unchanged.

This deliberately chooses fewer, longer attempts. A dropped request can now take 12s rather than 5s to recycle, and there are two retries instead of three; in exchange, the broker survives the slow-backend case production is currently exhibiting while retaining recovery from two transient stalls when the aggregate budget permits them.

Diagnostics and adaptation

Returned HTTP or transport errors still surface immediately with their concrete cause. Only deadline exhaustion takes the new path, which now states that no response arrived before the per-attempt deadline, includes total elapsed time and the aggregate limit, and warns that registration may still have completed after the client stopped waiting. That avoids turning an unknown outcome into a false unreachable diagnosis.

I did not add an adaptive deadline. Learning from an elapsed registration would require keeping an ambiguous, mutating request alive after timeout or adding a separate latency probe; that is not a small or obviously safe change. A fixed, tested budget is clearer here.

Validation

  • regression proof against the original defaults: the new 9.5s success test failed as intended, exit 101
  • regression proof against uncapped membership scaling: the new two-membership budget test failed as intended, exit 101
  • regression proof against the one-second preparation reserve: the new post-announcement reserve test failed as intended, exit 101
  • mise exec -- cargo fmt --all -- --check: pass
  • mise exec -- cargo test -p agent-relay-broker runtime::session::tests: 7 passed
  • mise exec -- cargo test -p agent-relay-broker: 1,014 passed, 4 ignored; all integration and doc tests passed
  • git diff --check: pass
  • strict all-target Clippy found one pre-existing unrelated while_let_loop lint at crates/broker/src/terminal_control.rs:1224; allowing only that baseline lint, all-target Clippy passed

Rollout note

AGENT_RELAY_HANDSHAKE_TIMEOUT_MS and AGENT_RELAY_HANDSHAKE_ATTEMPTS are per-process environment variables. Applying overrides fleet-wide today requires restarting every node, and restarting a node kills every agent on it. The default therefore matters more than the override: nodes receive the safer budget on their next ordinary restart without an operator remembering an environment variable.

Refs #1562