[kriszyp]

⊙ Problem

mqtt-packet sends reasonCode to a v5 client and returnCode to a 3.1.1 client. The CONNACK refusals in server/mqtt.ts had the two swapped, so a v5 client with a wrong password was told 4 and a 3.1.1 client 0x86. An error.code thrown during connect, such as an HTTP status, was also written as is. #3303 fixed SUBACK, PUBACK and DISCONNECT, not CONNACK. Refs #3298.

🤔 Is this the right problem?

Yes. Clients act on this code: a v5 client that gets 4 cannot tell bad credentials from a protocol error.

💡 Solution

• Every refusal now carries both codes (CONNACK_REFUSALS): bad credentials 0x86 / 4, not authorized 0x87 / 5, anything else 0x80 / 3.
• A throw from authorizeClient (the usual way a hook refuses) defaults to "not authorized" (its own catch); other connect failures are "unspecified".
• A numeric error.code is sent as is only if the client's version allows it in a CONNACK (connackRefusalFor). Otherwise 401 or 403 (statusCode or code) is "not authorized", and the rest takes the default. The refusal sites now use these (bad credentials, session errors).

🤔 Unspecified is 3 for 3.1.1

3.1.1 has no generic refusal. For a session or storage failure, 5 would suggest a credentials problem. 3 ("server unavailable") is what a session takeover timeout already sends. A hook refusal is a denial, so it keeps 5.

🤔 error.mqttReasonCode is not read at CONNACK

Only the inbound QoS 2 path sets it, and never during CONNECT. Reading it too would add a second way to name a code with no caller.

✅ Verification

• Node 22/24/26 unit jobs fail evictionBlobs.test.js:164, also failing on main. Node 24 additionally fails transaction.test.js:507, the same additional-audit-ref assertion present in main's Node 26 job. All job logs are now available; these failures are outside this diff.
• CI: runLinter and Format Check passed on ed89e94ce.
• npm run lint:required, npm run check:design-docs, and changed-file Prettier pass on ed89e94ce. The upstream design-note trim from merged Adopt rocksdb-js getRegistryStatus() in place of the deprecated registryStatus() alias #3429 restores the document to 998 lines. This maintenance commit leaves runtime code unchanged.

MQTT implementation validation from the authoring and rebase runs

• Unit: 41 cases in mqttNotAuthorized.test.js. 17 fail on origin/main (built side by side), all CONNACK cases, and all pass here.
• End to end: 4 new CONNACK cases in mqtt-not-authorized.test.ts over a real WebSocket client, using a fixture hook that refuses client ids starting denied- with a 403 (config); all 4 fail on origin/main (actual: 4, expected: 134). integrationTests/mqtt/**: 59 pass.
• unitTests/server/**: 1451 pass before the rebase; the three MQTT unit files pass after it. tsc, oxlint and prettier are clean.
• test:unit:main hit its 50 minute limit on this host before finishing. The 47 failures it had logged are in deploy-activation and EntryHandler suites, none touching MQTT. The MQTT unit files and the rest of unitTests/server/** pass.
• PR QA: skipped @ 50111f6 — no worker token available to queue it.

PR QA: skipped @ ed89e94 — documentation-only maintenance.

🤔 Why include the design-doc trim?

It is identical to merged main #3429. On this branch the linter fails at 1,014 lines. The separate trim commit fixes CI and keeps the PR's history. The 1,000-line gate remains intact.

⚖️ Alternatives

🤔 Keep the existing hook policy

Numeric hook codes stay version-specific, as the explicit tests require. Translating them across versions would change that policy. The cost of one refusal closure per CONNECT is unmeasured. This lint repair keeps that existing behavior. Post-hook 401/403 coverage and comment cleanup remain follow-ups. The async-hook and socket-close deferrals below remain separate.

🤔 Keep the published CONNECT handler

A single CONNECT catch with a hook-stage fallback could replace the shared handler. Both can preserve refusal semantics. This maintenance commit only restores the doc budget, so it retains the published runtime implementation.

_{🤖 Generated by Anthropic Claude and OpenAI Codex; posted via @kriszyp.}

https://claude.ai/code/session_01BeYyDXfTjDKHwpzUXyTHDZ

Release target: v5.3 — fixes code from #3303, a backport to v5.3
Related PRs: #3446 overlaps, #3464 overlaps, #3429 overlaps, #3430 overlaps, #3221 overlaps, #3456 overlaps, #3423 overlaps, 5 others independent
Complexity: medium

_{Dispatch: task harper-mqtt-connack-reason-codes · queued by unknown · ran by claude/sonnet/high · worker kzyp-xps-1}

Deferred findings

• b9c349471e41a5f6 minor security — server/mqtt.ts — Async authorizeClient rejection can admit CONNECT — NOT FILED
• 55167bb87455efd4 minor correctness — server/mqtt.ts — Refusal CONNACK leaves the connection open — NOT FILED

_{Review-Coverage: authored=codex; ran=gemini,claude,cursor-grok; adjudicated=domain; declined=cursor-composer,muse; rounds=6; full=2 @ ed89e94}

_{Review-Attention: study ~9m (hot: mqtt.ts; decisions: hook-fault-policy, do-less-alternative, bare-hook-error-is-denial, generic-legacy-refusal) @ ed89e94}

[github-actions]

Release cherry-pick v5.3: conflict

Cherry-pick onto v5.3 produced conflicts on commit(s): ed89e94ce33d99570d7da5d0a9999a22d7f198da

The conflict markers are committed on branch cherry-pick/v5.3/pr-3402.
A pull request has been opened to land this patch: #3555

[gemini-code-assist]

Code Review

This pull request updates the MQTT server implementation to correctly return protocol-specific connection refusal codes (reason codes for MQTT v5 and return codes for MQTT v3.1.1) during connection establishment (CONNACK). It introduces helper utilities to map internal errors to appropriate protocol codes and adds comprehensive integration and unit tests to verify these behaviors. There are no review comments, and I have no feedback to provide.

[kriszyp]

Suggestion: add a read-only-node arm to the CONNACK test.

On main c12239b4c, a read-only refusal reaches the CONNECT catch (server/mqtt.ts:536-550) with the string error.code === 'ERR_NOT_SUPPORTED', which mqtt-packet cannot encode (Invalid return code), so the client sees a DISCONNECT and a timeout. With read-only 405 refusals (#3427) the error has no .code, so the current fallback sends 5 on v5 (a v3.1.1 code reused) and 128 on v3.1.1. connackRefusalFor as described here should cover both, since it sends a numeric code only if valid for that version and otherwise 0x80 (v5) or 3 (v3.1.1). A read-only arm (v5 and v3.1.1 clients, both the string-code and the code-less error) would pin that.

The residual behavior, durable sessions (clean: false, QoS above 0) failing to subscribe or resume on a read-only node, is a separate design question tracked in see #3460.

— Claude Opus 5.5 (finding triage)