Wait for the restart a component deploy triggers, and tell MQTT clients why a publish was refused

[kriszyp]

deploy_component { restart: true } fired the worker restart without waiting for it and reported success immediately, so on platforms where replacements share a port (SO_REUSEPORT) every not-yet-replaced worker kept accepting connections while still running the pre-deploy component set — a client acting on that success was served, per connection at random, by a worker that had never heard of the component it just deployed. On MQTT that surfaced as a publish refused with a bare 0x80 "Unspecified error" and then, when the worker was finally replaced, an unannounced socket close (harper#2335). The deploy now waits for the restart, an unhandled-topic publish is refused with 0x90 "Topic Name invalid" and a reason string, and a worker that is shutting down sends MQTT v5 clients DISCONNECT 0x8B "Server shutting down" instead of dropping the socket. Measured on Linux with 4 worker threads: 38 of 870 publishes refused across the first ~3s before, 0 of 876 after.

Docs companion: HarperFast/documentation#638. The pool-availability throttle change that was here now ships separately as #2363.

For the human reviewer

1. The restart outcome is logged, not returned. An earlier revision added a restart_completed response field; it only spoke to the exceptional cases (the wait gave up, a worker could not be replaced, the process was already shutting down), all of which the log names, and it would have been public API we could not narrow later. So a successful deploy means what the await gives it and nothing more. The cost: a caller cannot distinguish "restarted" from "restart still running" programmatically, and this PR's own test lost a deterministic guard against the wait silently stopping early (the publish fan-out still catches it, less pointedly).
2. A peer applying a replicated deploy does not converge before responding, and cannot with this design: restartWorkers() from a worker only hands the restart to its main thread, and the worker that would await it is itself about to be restarted and sever the connection it would answer on. Making a peer report a finished restart needs the originating node to track a job rather than an operation response — recorded here as the known limitation, not attempted.
3. The wait follows the restart's progress instead of a clock. No single timeout is right: a replacement has its own multi-minute startup backstop, a component install reports nothing for as long as npm takes, and a draining worker may hold its exit for the whole drain ceiling. So restartWorkers() reports each worker it replaces, beats while root components load, and passes on the deadline a draining worker asks for; the wait gives up after 60s without progress, with a 10-minute absolute ceiling. The first version was a flat 120s cap, which the PR review correctly called out as reporting healthy wide-pool restarts as incomplete. None of the three numbers is configurable — a caller who wants an asynchronous restart already has restart: "rolling" and its job id.
4. A restart that throws no longer fails the deploy. awaitRestart turns a rejection into { completed: false } and logs it, so a restart that threw outright is reported like one that merely ran long. The alternative — failing an operation whose component is already on disk and replicated, with no rollback — seemed worse. One line to reverse.
5. drop_component { restart: true } now waits the same way. It is the same race in the removal direction (a dropped component's topic still served by a not-yet-replaced worker), so leaving it asymmetric looked worse than the extra surface; it is a behavior change on a second operation, though, and out of the issue's literal scope.
6. Residuals, all test coverage. Nothing drives a real worker drain through extendTerminateDeadline into a held-open deploy, nothing proves a drop-with-restart waits until the removed topic is gone from every worker, nothing publishes QoS 1 while a shutdown lands, and nothing proves an error this layer does not map has its message withheld (the safety half of the reason-string change). Each needs a fixture or a minute-scale test; the mechanisms they cover are exercised by the four integration cases here plus the ten awaitRestart unit cases.

Also worth a look: the client notification rides the ShutdownDrain registry with hasWork: () => false, a mechanism whose stated purpose is extending the shutdown deadline for in-flight work. It runs before closeServers(), which both moves MQTT teardown slightly earlier than where an in-flight publish previously completed and delays the port release a non-SO_REUSEPORT replacement is waiting on (the hazard already documented for drains in integrationTests/components/shutdown-drain-e2e.test.ts). A dedicated post-closeServers hook would avoid both, at the cost of a second lifecycle hook. Separately, the no-resource error is tagged statusCode = 404 on the session so any protocol layer can map it — an ad-hoc property on a plain Error that is now a cross-protocol contract rather than a shared error class.

Verification

New integration suite integrationTests/mqtt/deploy-restart-topic-availability.test.ts (Node, 4 worker threads, MQTT over WebSocket): the deployed topic is live on every worker as soon as the deploy returns; an unhandled-topic publish is refused 0x90 with a reason string; a client that sent requestProblemInformation: false gets the code without the string; a restart sends v5 clients DISCONNECT 0x8B while a v3.1.1 client is only closed. Fails-on-base against origin/main sources with a forced rebuild, for the right reasons: 32 of 32 publishes refused, expected reason code 0x90, got 128, no DISCONNECT received before the socket closed.

unitTests/components/awaitRestart.test.js covers the wait's outcomes with injected timers: counts passed through, handoff, a declined restart, rejection, a synchronous throw, stall, progress keeping it alive, a reported deadline outliving the idle window, the absolute ceiling, and no timer left armed after it settles. Four assertions were checked against deliberately broken builds — removing the settled guard, moving startRestart out of the promise chain, restoring the !== 0 problem-information check, and dropping the v5 gate on the shutdown DISCONNECT each turn one red. That third one caught a real bug in this PR's first version: mqtt-packet parses the CONNECT byte into a boolean, so comparing it to 0 never suppressed anything.

Before/after on the probes that drove the diagnosis (Linux, 4 threads, one node): fresh connection per publish, 38 of 870 refused over the first ~3s before the change, 0 of 876 after; 8 connections pinned before the deploy and publishing across the restart, 14–20 QoS 1 publishes never settled with zero disconnect events before, every client receiving reason code 139 before close after.

Suites run on Linux: integrationTests/{deploy,mqtt,components} 195 passed / 0 failed / 1 skipped, and integrationTests/{deploy,mqtt,components,apiTests} 1123 passed / 0 failed / 14 skipped a few commits earlier; integrationTests/server 0 failed (6 Ollama-backend cancellations, no local daemon). CI was green on all 44 checks before origin/main was merged in. Two runs on that Linux box showed unrelated failures — 9 job timeouts in apiTests/terminology.test.mjs, one Harper startup watchdog in components/static-after-rest.test.ts — neither of which reproduces in isolation (48/48 and 7/7) or on an idle box; it also hosts the dev-agent fleet.

The reason string is sent only for the failures this layer maps to a code of its own (statusCode 403/404), is trimmed and size-checked in encoded bytes, and is withheld when the client's CONNECT set requestProblemInformation: false or advertised a maximumPacketSize too small for it. The remaining client-side hang after a shutdown is mqtt.js's own behavior — it flushes pending publish callbacks only on a client-forced end() with reconnectPeriod: 0 — so the DISCONNECT is asserted, not the settling of an in-flight publish.

Complexity: medium

_{Review-Coverage: authored=claude; ran=codex,gemini; declined=cursor-grok,cursor-composer,domain; rounds=17 @ e2826aa}

_{Human-Review-Need: 4 @ e2826aa}

[gemini-code-assist]

Code Review

This pull request improves the reliability of worker restarts during component deployments, particularly for MQTT topic availability. It ensures that deployments await worker restarts, gracefully disconnects MQTT clients with a 'Server shutting down' reason code during restarts, and maps publish errors to specific MQTT reason codes. Feedback on these changes suggests making the hardcoded 120-second restart timeout dynamic or configurable to accommodate larger thread pools under load, and wrapping property accesses on caught errors in a try/catch block to avoid potential crashes from untrusted objects.

[claude]

Reviewed; no blockers found.