The Handshake That Needed a Follow-Up

desertaxle merged PR #22004 — 1,455 lines across 8 files to give Prefect’s self-hosted workers a proper WebSocket handshake. Worker says hello, server says ready, heartbeats flow through the socket instead of REST. A reasonable protocol upgrade. It just shipped with a bug the developer designed into the protocol himself.

The commit history tells the whole story. It opens with “Add OSS worker channel handshake” and immediately enters remediation: “Fix worker channel PR checks,” “Fix worker channel CI failures,” “Address worker channel setup review comments,” “Address worker channel event review comments,” “Narrow worker channel PR scope,” “Fix worker channel teardown race,” “Adjust worker tests for channel handshake.” One feature commit. Seven fixups. The “Narrow worker channel PR scope” commit is particularly telling — it means the PR was too ambitious even by the author’s own standards and had to be cut down mid-review. The git log reads less like development and more like negotiating with your own code.

The core of the handshake lives in _build_worker_ready_frame, a function in workers.py that assembles the server’s response to a connecting worker. It resolves the work pool from the database, refreshes it after a possible template update, records a heartbeat, then refreshes the pool a third time to build the initial snapshot. Each read is guarded by a bare assert something is not None — three round-trips, three asserts, in a production FastAPI WebSocket handler. These aren’t test assertions. They’re in the hot path of every worker connection. If the database ever returns None here, the helpful assertion message disappears the moment someone runs Python with -O, and your ops team gets a raw AttributeError on the next line instead.

The pièce de résistance is the worker_id field. The ready frame sends back "worker_id": worker.id, cheerfully handing every connecting OSS worker the server’s internal database UUID. Three days later, PR #22067 arrived to change it to None. The REST heartbeat endpoint — the thing this WebSocket channel was explicitly designed to complement — has never exposed worker IDs in OSS. The field was typed Optional[UUID] in the protocol contract that desertaxle himself wrote. He looked at Optional[UUID], decided “I’ll always fill this in,” and nobody caught it until the code was on main and leaking internal IDs into worker environment variables and log payloads. The fix was one line.

Credit where it’s due: the race-condition handling for concurrent pool creation is properly done. begin_nested() savepoint, IntegrityError catch, re-read fallback — textbook defensive database programming that shows real understanding of transactional edge cases. It’s just darkly comic that the same developer who anticipated two workers racing to create a pool couldn’t anticipate what “Optional” means in his own protocol frame.

Final Verdict: The Protocol Overcommitter — ships the handshake, forgets what it promised