fix(server): socket noDelay/keepAlive options were never applied on TCP/UDS listeners, and keep-alive delay was 600ms not 10min

[kriszyp]

Summary

onSocket() in server/threads/threadServer.js created its plain-TCP listener and the TLS UDS mirror with net.createServer(listener, options). Node's signature is net.createServer([options][, connectionListener]) — when the first argument is a function it becomes the connection listener and the second argument is silently ignored, so noDelay/keepAlive/keepAliveInitialDelay were never applied on those servers. Fixed by swapping to options-first.

While auditing the other listener sites, a second latent bug turned up: keepAliveInitialDelay is in milliseconds and net.Server floors it to whole seconds (~~(ms/1000)), so the value 600 — commented as "10 minute keep-alive" — floored to a 0-second initial delay everywhere it was applied (the TLS listener and both HTTP server sites). In practice that meant SO_KEEPALIVE enabled with the OS-default idle time (~2 h on Linux), not 10 minutes. Changed to 600_000 at every site.

Since the trio was repeated at five sites and had already drifted, it now lives in one shared socketOptionDefaults export in serverRegistry.ts (a leaf module both http.ts and threadServer.js already import).

Behavior changes to weigh (this is the part worth reviewing)

Options that were inert for years now take effect:

• Plain-TCP component listeners (e.g. MQTT 1883): Nagle is now disabled and TCP keep-alive is enabled (probe after 10 min idle). Healthy idle connections are unaffected — probes are ACKed by any live peer; only true half-open connections (peer crashed, NAT/firewall dropped the flow) get reaped, which was the original stated intent. This also removes a long-standing asymmetry with the TLS listener (8883), which always had noDelay.
• TLS listener + HTTP/HTTPS servers + HTTP UDS mirror: keep-alive idle tightens from OS default (~2 h) to 10 minutes.
• UDS mirrors: the options are no-ops on AF_UNIX handles (net.js applies them only when the handle supports setNoDelay/setKeepAlive), so no behavior change there — fixed for correctness and consistency.

Verification

• Empirically confirmed both bugs and the fix against Node 24: reversed args leave server.noDelay/keepAlive false; 600 floors to 0 s; 600_000 yields setKeepAlive(true, 600) on the accepted socket's handle.
• New regression test in unitTests/apiTests/mqtt-test.mjs asserts the three server.socket()-created listeners (TCP, TLS, and the UDS mirror via tls_unixDomainSockets) actually carry the options. Verified it fails against the old code and passes with the fix.
• MQTT apiTest suite: identical pass/fail set before vs. after the change locally (the local failures are pre-existing environment issues, also present on main); udsMirror.test.js 26/26 passing.

Notes

• Merge overlap with PR #1858 (kris/pp2-mtls): it touches the same UDS-mirror call site (wrapping the listener in withProxyProtocol(listener)). Whichever lands second has a small, mechanical conflict — the listener argument (wrapped or not) just stays in the second position.
• Cross-model reviews: Codex — no findings; Gemini — no blockers, one dedup suggestion (adopted as socketOptionDefaults).
• Generated by an LLM (Claude Fable 5), reviewed and verified as described above.

[gemini-code-assist]

Code Review

This pull request centralizes socket hygiene defaults (such as noDelay, keepAlive, and keepAliveInitialDelay) into a single exported object socketOptionDefaults to prevent configuration drift across different listener types. It also corrects the keep-alive initial delay from 600ms to 10 minutes (600,000ms) and fixes an argument order bug in createSocketServer calls. A new unit test is added to verify these options. The reviewer suggests using assert.strictEqual instead of assert.equal in the new test to comply with strict assertion standards.

[claude]

Reviewed; no blockers found.

[cb1kenobi]

Nice find!