Fix MQTT UDS listener exporting an empty TLS certificate list to a fronting proxy

[kriszyp]

Summary

Fixes #1998. A raw-socket TLS listener's certificate selector (used by MQTT's network.securePort) could permanently end up with an empty certificate map, so a fronting proxy (Symphony) that terminates TLS and reads Harper's per-socket exported cert metadata to route by SNI saw an empty certificates: list and served the instance's own node certificate for every connection to that port.

Confirmed live on 2 of 3 nodes of an affected production cluster: every worker's exported MQTT UDS metadata had an empty certificates: list, unchanged since container boot, while HTTP-based listeners on the same workers were fully populated.

Root cause (corrected 2026-07-30 after live investigation)

The original diagnosis — hdb_certificate.subscribe() throwing before updateTLS() ran, rejecting .ready — was falsified on the affected node: the yaml existed (written twice, ~2 h apart, both with valid pid/tid/port and empty certificates:), which requires updateTLS() to have completed both times, and neither predicted error signature appears anywhere in hdb.log (retention back to May).

What the evidence supports instead: the node was mid live v4.x→v5.1 upgrade, and the table-by-table LMDB→RocksDB migration migrated hdb_certificate at a time exactly bracketing the second empty write. An engine migration replaces databases.system.hdb_certificate with a brand-new table object (the hazard documented by schemaMigrationFragility.test.js "F4"), and the bulk row-copy bypasses the subscribe/event path entirely. The selector's cert-table subscription had bound to the old object, so it was orphaned by the table swap: two completed-but-empty passes during the transition, zero exceptions, and total silence afterwards even with 7 valid cert rows in the table.

The fixes here cover that mechanism directly:

• subscribedTable instance tracking re-subscribes whenever databases.system.hdb_certificate is a different object than the one subscribed to (with teardown of the orphaned subscription).
• The zero-certificates retry refuses to publish/resolve a completed pass that produced an empty context map (the exact artifact observed), retrying on the debounce instead.
• The original readiness guard (don't touch/resolve when the table isn't loaded yet) remains as hardening for the boot-order race it was written for — that race is real even if it wasn't this incident's mechanism, and .ready must stay pending rather than let Bun's listener path start plaintext.

Scope (split per review)

Review feedback flagged that two parts of the original PR were blocking a near-clean v5.1 patch cherry-pick:

• The usageType/'server'-allowlist half (MQTT getting its own TLS usage type) → moved to feat(tls): give MQTT's raw-socket listener its own TLS usage type (split from #1999) #2003. No-op for stock deployments; v5.1 predates getEffectiveTlsCiphers, which is where the wide conflict came from.
• The unrelated auditLog.test.js deflake → moved to test: deflake audit log subscription-event assertion (split from #1999) #2002.

What remains here is exactly the customer-facing fix, which applies to v5.1 as-is.

Where to look

• security/keys.ts — createTLSSelector's readiness guard and retry logic, the cert-table subscription lifecycle (instance tracking, teardown, failure reset), the per-row cert parse guard, and the zero-certificates retry (gated on liveReload so getReplicationCert's one-shot existence check — which the cert bootstrap flow relies on resolving empty — can't hang).
• unitTests/security/keys.test.js — race tests (both the whole-system-missing and hdb_certificate-not-attached cases), zero-certs retry/transient-resolve tests, unparseable-record test.

Review history (five rounds — each surfaced real, fixed issues)

Round 1 (Gemini + Codex, cross-model-review skill, thorough mode):

• [Codex, blocker, fixed] First pass at the retry branch called resolve() on .ready before real certs were available — Bun's listener path would have silently started a secure listener as plaintext, permanently. Fixed: the retry branch no longer resolves.
• [Gemini, blocker, fixed] Guard checked databases.system === undefined, missing databases.system present but .hdb_certificate not yet attached. Now checks databases.system?.hdb_certificate.
• [Codex, significant, fixed] usageType: 'mqtt' compat break for uses: ['server'] certs — now in feat(tls): give MQTT's raw-socket listener its own TLS usage type (split from #1999) #2003.

Round 2 (Harper-domain pass): 'server' legacy-match scoping (operations-api leak) — now in #2003.

Round 3 (PR review — gemini-code-assist + human review):

• [gemini-code-assist, fixed] Boolean subscribe flag → subscribedTable instance tracking (which, per the corrected root cause above, turned out to be the load-bearing fix).
• [gemini-code-assist, fixed] Subscribe gated on liveReload so transient selectors don't pin scheduleRebuild onto the table's subscriber list.
• [human review, fixed] Denylist → allowlist for the 'server' fallback — now in feat(tls): give MQTT's raw-socket listener its own TLS usage type (split from #1999) #2003.
• [human review, fixed] Race test rewritten from Sinon fake timers/rewire internals to the real module + real timers + waitFor().

Round 4 (experimental Grok leg on the post-round-3 diff):

• [fixed] Race test leaked a permanent liveTLSRebuilders registration (liveReload defaulted true); now passes false.
• [fixed] node:assert/strict → plain node:assert per repo convention.
• [fixed] Two coverage gaps: recovery tests now assert secureContexts is actually non-empty, and the system-present-but-table-missing case is covered.

Round 5 (PR review — Devin-Holland/Claude verification pass + dispatch fix):

• [fixed] Silent retry → latched breadcrumb log (tlsSelectorWaitedForSystemDb).
• [fixed] subscribedTable left pointing at a table whose subscribe() rejected → reset in the .catch.
• [fixed] One unparseable certificate row aborted the whole pass before any context was built → per-row guard in the CA-collection loop.
• [fixed] A completed pass resolving zero certificates still published the empty list (the observed artifact) → retry instead, gated on liveReload because getReplicationCert()'s bootstrap existence check must keep resolving empty immediately (an unconditional retry would hang first-boot cert generation — caught by tracing callers, covered by a dedicated test).
• [review's root-cause challenge, confirmed] The mechanism mismatch flagged in that review was verified on the affected node and led to the corrected root cause above.

Known caveats (deliberately not changed further)

• Cipher/@SECLEVEL can freeze against a "not-yet-loaded" state if the boot race hits. getEffectiveTlsCiphers() is still called once, synchronously, at socket-construction time. If the race occurs, the listener's cipher policy is resolved against an empty cert/CA set and won't change after the selector recovers (the code warns on a later mismatch but doesn't re-create the listener). Pre-existing; independently flagged by Codex (R1), Grok (R4), and the R5 review (which also noted recovery makes it slightly more reachable — pre-fix a stranded selector never served anything). A real fix needs deferred cipher resolution; separate change.
• The retry never gives up. If the table genuinely never loads (or legitimately never yields a cert), .ready stays pending forever; on Bun, listenOnPortsBun() awaits it in its per-port loop, so the pathological tail stalls later ports. Flagged in R5; the breadcrumb log makes this diagnosable. Accepted as the safer failure mode vs. publishing empty/plaintext.
• Zero-certs retry vs. a truly cert-less listener type: a live selector for a type with genuinely zero matching certs now retries on the debounce instead of resolving empty. In practice a node always has at least its self-signed default cert once bootstrap completes; the transient (liveReload=false) path is exempt.

Testing

• unitTests/security/keys.test.js: 54/54 passing, including the exact-race tests (both missing-system and missing-table variants), the zero-certs retry + transient-resolve pair (boot-time and post-success transient rebuilds — the latter bisected against the disarmed-guard version), the table-swap resubscribe regression test (bisected against the boolean-flag version), the unparseable-record skip, and the transient-selector leak fix.
• npm run build, npm run typecheck:fast (no new errors — 4 pre-existing, unrelated on main), prettier --check, oxlint clean on changed files. unitTests/security/**: 390 passing; unitTests/server/udsMirror.test.js: 35 passing.

Generated by Claude (Sonnet 5 / Opus 5), with Kris driving the live investigation and review.

[claude]

Reviewed; no blockers found.

[gemini-code-assist]

Code Review

This pull request updates the TLS selector and cipher relevance logic to support legacy 'server'-tagged certificates for non-HTTP listeners (like MQTT) while excluding 'operations-api'. It also addresses a boot-order race condition where the system database is not yet loaded when a listener is initialized, ensuring the .ready promise remains pending instead of resolving early or throwing. The review feedback highlights two critical issues in the subscription logic: a potential memory leak in transient selectors (when liveReload is false) and a loss of subscription during database resets. To resolve these, the reviewer suggests tracking the actual subscribed table instance and checking the liveReload flag before subscribing.

[github-actions]

Patch cherry-pick: conflict

Cherry-pick onto v5.1 produced conflicts on commit(s): bcf9cb9055467e7bf9f9c7bb857a2b6d6e300a7c ec80172a5684c30de1caf757a3baf0ab47b4d527 051c75271f09369ffb2bc393f3cdbcca2b609027 ffb161d71b11d0d70935d20f8c56c56e19d7f29d a306d0bef79621a95a8bf98b50b15b227954bf84 a7baa4349254b150c53aa83dd52452da9e76ae77 13ed49f936d991646da4bc14e3b3359bc2fdb12f

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

[Devin-Holland]

Verified

Ran this locally on a fresh worktree of kris/fix-mqtt-uds-cert-export (node 24, clean npm ci + tsc --project tsconfig.build.json):

• Bisect confirms the tests guard the fix. With security/keys.ts restored to origin/main and everything else on the branch, exactly the three new tests fail — with the predicted messages (.ready must stay pending while the system database has not loaded, the hdb_certificate-not-attached variant, and expected undefined to equal 'DEFAULT@SECLEVEL=0' for the allowlist test) — and 47 others pass. Restored: 50/50.
• unitTests/security/** + unitTests/server/udsMirror.test.js: 422 passing, 0 failing.
• unitTests/resources/auditLog.test.js: 3/3 runs green, the reworked assertion completing in 51–69 ms against its 500 ms-per-write budget.
• prettier --check and oxlint clean on all five changed files.
• The recovery path is end-to-end real. updateTLS() fires server.secureContextsListeners before resolve(), onSocket() pushes writeMetadata onto that list, and host-manager's _createSocketWatcher() watches the sockets dir (500 ms debounce) and re-reads entries — so a yaml written late by the retry does propagate into Symphony's routes without a restart.

The readiness guard, the subscribedTable-identity fix, the liveReload gating, and the previous-subscription teardown all look right to me, and the reasoning in the comments is the kind of thing I wish more of this file had.

One thing I'd want confirmed before we tell the customer it's fixed

I can't reconcile the diagnosed mechanism with the reported artifact, and they look mutually exclusive.

The diagnosis is that databases?.system.hdb_certificate.subscribe(...) throws before updateTLS() runs, so .ready rejects. But writeUdsMetadata() has exactly three call sites, and every one of them is a writeMetadata closure reached only via SNICallback.ready.then(...) or server.secureContextsListeners — and secureContextsListeners is invoked from exactly one place, updateTLS() itself (security/keys.ts:1166). So on the throwing path neither fires, and no yaml is written at all. The reported artifact is a yaml that exists with an empty certificates: list, which means updateTLS() ran to completion with an empty secureContexts — the guard this PR adds would never have been reached.

Two more things point the same way:

1. host-manager's _buildCertConfig() has carried this comment since the original Symphony commit (db606a7, 2026-06-01): "…or ships an empty certificate list (Harper's MQTT secure socket, whose secureContexts aren't populated when the metadata is written)". That fallback — presenting keys/fullchain.pem for every SNI — is exactly the customer symptom, and it's been the documented expected state of the MQTT socket for two months, on every node, not a race that landed on 2 of 3.
2. If the yaml were missing, _readSocketEntries() would return no entry for -8883.yaml, so there'd be no UDS route for 8883 at all — a different symptom than "wrong cert".

An alternative that produces the observed artifact exactly: updateTLS() completes, but every certificate is skipped by the inner catch at security/keys.ts:1143 — most plausibly getPrivateKeyByName() throwing ENOENT on the readFileSync fallback when privateKeys isn't populated on that thread yet. That empties secureContexts without throwing, the yaml gets written empty, and it never self-heals: the cert-table subscription is registered but omitCurrent: true means only a future cert-table change retriggers it, and private keys arriving isn't one. This PR's guard doesn't cover that.

Cheap discriminators on an affected node, both of which you have access to and I don't:

• Does sockets/<worker>-8883.yaml exist at all on an affected worker (vs. missing)?
• Does the worker log carry Unhandled promise rejection … Cannot read properties of undefined (reading 'hdb_certificate') at boot (the diagnosed path), or Error applying TLS for <cert name> … at error level (the alternative)?

If it's the latter, this PR is still a good hardening of a real latent bug — but the customer's issue would still be open. The inline suggestion on the retry branch is partly aimed at making that question answerable from a log next time instead of from a live cluster.

Suggested scope split for the v5.1 patch

origin/v5.1 has no getEffectiveTlsCiphers / ciphersCandidateRelevant / resolveEffectiveTlsCiphers at all — its onSocket() still reads tlsConfig.ciphers ?? tlsConfig[0]?.ciphers inline. That's why the cherry-pick conflicts are so wide (a single 150-line HEAD vs. branch block in keys.ts, plus threadServer.js and both test files).

Meanwhile the usageType half is a no-op for every stock deployment: Harper only ever writes uses: [], ['operations-api'], or ['replication'] (security/keys.ts:274, :455, :464, :704, :725), and for all three of those, both ciphersCandidateRelevant() and the quality scoring give identical results for type = 'server' and type = 'mqtt'. A 'server'- or 'mqtt'-tagged cert can only come from hand-written tls config.

So the half that's causing the v5.1 conflicts is the half that changes nothing for anyone today, and the half the customer needs (the readiness guard + subscription lifecycle) applies to v5.1 as-is — v5.1 has the identical pre-updateTLS() subscribe() call. Landing them as two PRs would make the patch cherry-pick nearly clean and shrink what has to be justified for a customer hotfix. Same for the auditLog.test.js change, which is one of the three conflicting commits and unrelated to TLS.

Minor

• The retry branch clears the module-level shared caCerts map (:996) and returns without repopulating it. Harmless in practice — every updateTLS() pass clears and rebuilds it before use, and the live availableCAs reference is never read anywhere — but hoisting the two guards above the .clear() calls would make the retry path non-destructive by construction.
• Cipher/@SECLEVEL freezing against a not-yet-loaded cert set (your second known caveat) gets slightly more reachable now that the selector recovers instead of staying dead: pre-fix a stranded selector never served anything, so a stale cipher string was moot. Agreed it's out of scope, but it may deserve its own issue rather than only a PR-body note.
• The retry never gives up, so if the table genuinely never loads, .ready stays pending forever. For onSocket() that's benign, but listenOnPortsBun() (server/threads/threadServer.js:426) awaits config.tlsSelector.ready inside its per-port for loop, so a permanently-pending .ready there stops the loop, skips every later port, and never posts CHILD_STARTED. Pre-fix that was a caught-and-logged per-port failure. Normal boot resolves within a tick or two of the debounce, so this is the pathological tail only — but it's another reason the retry deserves a log line.

Reviewed by Claude (Opus 5) at Devin's request — flagged as customer-critical, so I leaned on empirical checks over reading. Nice work on the round-by-round writeup; it made this much faster to audit.

[Devin-Holland]

Two more, both terse. Also a correction to my earlier body: I cited host-manager's _buildCertConfig fallback as evidence the exported list was empty — that fallback also fires for a populated list with no SNI match, and deliberately for the keyPath === privkeyPath case, so it doesn't distinguish the two. The rest of that comment stands: _readSocketEntries enumerates *-8883.yaml, so any UDS route at all requires the yaml to exist, which requires updateTLS() to have completed.

Claude (Opus 5)

[kriszyp]

Addressed 4 of the 6 open review threads (commit a9b808c):

• Silent retry breadcrumb (security/keys.ts:1018): added a latched logger.warn when the selector starts waiting for system.hdb_certificate to load.
• subscribedTable stranded on subscribe failure (:1038): the activeSubscription.catch now clears subscribedTable so a failed subscribe doesn't permanently strand the retry.
• Unparseable cert aborts the whole pass (:1045): the CA-collection loop now guards X509Certificate parsing per row and skips a bad record instead of throwing before any context is built.
• Empty secureContexts after a completed pass (:1002's sibling guard, ~:1173): extended the "not ready" treatment — if a completed pass resolves zero certificates, retry instead of publishing the empty list. Gated on liveReload: I found that applying this unconditionally would have hung getReplicationCert()'s one-shot existence check forever on a fresh install (the bootstrap flow that creates the first replication cert depends on that call resolving empty/falsy). Added tests for both the retry-then-recover path and the transient-selector-still-resolves-empty path.

Also hoisted the two readiness guards above secureContexts.clear()/caCerts.clear() per the "minor" note, so the retry path never wipes previously-published state.

Ran npm run build, typecheck:fast (same 4 pre-existing unrelated errors as main), prettier --check, oxlint, and unitTests/security/** (390 passing) + unitTests/security/keys.test.js (53 passing, up from 50) + unitTests/server/udsMirror.test.js (35 passing) — all clean.

Not addressed (leaving open — see the two remaining threads and my question on the PR):

• Splitting the usageType/cipher-scoring half and the auditLog.test.js commit into separate PRs to unblock the v5.1 cherry-pick.
• The open question on whether the diagnosed race actually matches the reported customer artifact.

Independent pre-push review (codex, opposite-family) did not complete — the machine has heavy contention from other concurrent dispatch reviews right now and it didn't finish inside the available time budget after 3 attempts (~30 min). Flagging this so it isn't mistaken for having passed; happy to re-run once load clears.

— Claude (Opus 5), dispatch fix-harper-1999

[kriszyp]

Follow-up on the two remaining threads, per Kris's rulings (commit 9ee2274 + two new PRs):

Root cause — corrected. The live investigation on the affected node answered the review's discriminators: the yaml exists (written twice, both times valid-but-empty) and neither predicted log signature appears anywhere in the log history. That falsifies the original subscribe-throws diagnosis. The evidence points to the cert-table subscription being orphaned by a table-object swap during the node's live v4.x→v5.1 upgrade — the LMDB→RocksDB migration of hdb_certificate brackets the second empty write exactly, engine migration replaces the table object (the schemaMigrationFragility "F4" hazard), and the bulk copy bypasses the subscribe/event path. The round-3 subscribedTable instance-tracking fix covers that mechanism directly, and the new zero-certs retry refuses to publish the empty artifact regardless of which path produces it. PR description rewritten accordingly (see "Root cause (corrected)").

Scope split — done.

• feat(tls): give MQTT's raw-socket listener its own TLS usage type (split from #1999) #2003 — feat(tls): give MQTT's raw-socket listener its own TLS usage type (the half that's a stock-deployment no-op and caused the wide v5.1 conflict).
• test: deflake audit log subscription-event assertion (split from #1999) #2002 — test: deflake audit log subscription-event assertion (rebased onto main, whose independent partial deflake it supersedes).
• This PR is narrowed to exactly the customer-facing readiness/subscription fix, which applies to v5.1 as-is. unitTests/security/keys.test.js: 52/52 after the split; build/typecheck/prettier/oxlint clean.

I resolved both threads; the inline reply endpoint was blocked by an existing pending review draft on this PR, so the replies live here instead.

— Claude (Opus 5), dispatch fix-harper-1999

[Devin-Holland]

Re-review of a306d0bef

All four suggestions landed faithfully, plus the guard hoisting. Re-verified from scratch on a clean npm ci + tsc --project tsconfig.build.json worktree (node 24):

• unitTests/security/keys.test.js — 52 passing; unitTests/security/** + unitTests/server/udsMirror.test.js — 426 passing, 0 failing; build 0 errors; oxlint and prettier --check clean.
• Both new warn lines fire on the real paths (saw waiting for system.hdb_certificate to load and resolved zero certificates; retrying in the run output), and the unparseable-record guard logs Skipping unparseable certificate record and continues rather than aborting.

Two things I want to credit before the finding:

• The getReplicationCert catch is a good save on my suggestion. I proposed the zero-certs retry without checking the bootstrap path; gating it on liveReload is right, and the negative test for it is the part I'd have missed.
• Thanks for running the discriminators. "The yaml exists, written twice, both valid-but-empty, and neither predicted log signature appears anywhere" is exactly the check that settles it, and rewriting the root-cause section rather than leaving the original narrative in place is the right call.

The corrected root cause is now the untested one — and I could not get it to work

The PR now names the mechanism as the cert-table subscription being orphaned by a table-object swap during the v4.x→v5.1 engine migration, with the subscribedTable instance-tracking as the fix. That comparison lives inside updateTLS, so it can only run when something calls updateTLS — and after a swap orphans the subscription, the candidate triggers are: the (now-dead) subscription, the retry timer (only pending if the last pass returned early), and rebuildLiveTLSContexts(), which has exactly one caller: handlePrivateKeyReload, gated on previous !== private_key.

I tried to demonstrate the recovery path and could not. Probe, against this branch:

const selector = keys.createTLSSelector('mqtt', undefined, true);
await selector.initialize(pseudoServer);            // contexts = 4, healthy baseline
let subscribeCalls = 0, searchCalls = 0;
databases.system.hdb_certificate = {                // model the engine-migration swap
    subscribe: async () => { subscribeCalls++; return { end() {} }; },
    search: () => { searchCalls++; return realTable.search([]); },
};
await sleep(4000);                                  // 2.6× TLS_REBUILD_DEBOUNCE_MS
// → subscribeCalls=0 searchCalls=0
for (const r of keys.__get__('liveTLSRebuilders')) r();   // set size is 1 — this selector's
await sleep(3000);
// → subscribeCalls=0 searchCalls=0   (still nothing)

Idle for 4 s: no re-subscribe, no re-read — expected, since nothing triggers. But firing the selector's own registered rebuilder directly also produced nothing, and I don't have an explanation for that. It may well be my harness (I did not chase it further), but I couldn't show updateTLS re-running after a swap by any route.

So the mechanism the PR now credits for fixing the incident is the one part with no test, and my attempt to exercise it by hand didn't show it working. That upgrades the existing "missing coverage" thread from a nice-to-have to the thing I'd want green before this ships to a customer. Please take the probe above as a starting point rather than a verdict — if it passes for you with subscribeCalls=1, my harness was wrong and a regression test pinning it down closes this out.

Worth noting what is not at risk: the customer-visible protection doesn't depend on this. The zero-certs retry refuses to publish the empty certificates: list whatever produced it, and I verified that independently. In the reported incident the pass resolved empty, so the retry timer stays live and would keep re-entering updateTLS — which is plausibly why the fix works there even if swap detection never fires on its own.

The general case is the gap: a swap that happens while secureContexts is non-empty leaves no pending timer and a dead subscription, so the selector would keep serving its current contexts and miss every later cert-table update — a renewal that only arrives via the table wouldn't land until a key-file change or a restart. Different symptom from this incident, same orphaned-subscription cause. Probably its own issue rather than more scope here.

Smaller

• The zero-certs warn isn't latched the way the system-db one is, so a genuinely stuck listener logs every 1.5 s — ~57k lines/day. Already covered by the open bot thread; just confirming the arithmetic.
• secureContexts.size === 0 is the right condition for "don't publish an empty list", but it isn't quite "no TLS available": defaultContext is assigned outside the per-hostname loop, so a cert whose hostnames resolves to [] (no SAN entries survive parsing and no CN — hostnamesFromCert returns [] there) sets a usable default context while leaving the map empty. That selector's .ready now never resolves, where before it resolved and served via the default. Narrow enough that I'd only note it, but && !defaultContext would make the guard say what it means.
• Noting for the record that the codex leg didn't complete, since you flagged it — no objection, just visible.

Reviewed by Claude (Opus 5). Verification commands and the probe are reproducible on a306d0bef.

[kriszyp]

Response to the re-review (commit a7baa43):

Ran your probe — the rebuilder-fired leg works here. Same shape as yours (healthy baseline → swap to a counting mock → idle 4s → fire the registered rebuilder → wait past the debounce): idle phase shows subscribeCalls=0, searchCalls=0 exactly as you saw, but after firing the rebuilder I get subscribeCalls=1, searchCalls=2. So the swap-detection does run when updateTLS is re-entered, and your harness was right to be suspected on that one leg. Your trigger analysis stands, though — the idle result is the true and important part: nothing spontaneously re-enters updateTLS after a swap when contexts are non-empty.

The regression test you asked for is in (re-subscribes to the new table instance...): real-module trigger chain (cert-table write → old subscription fires → debounced rebuild → swap detected), sentinel hostname so the assertion is selector-specific rather than racing the cross-selector counter burst (in the full suite, live selectors leaked by earlier tests also react to the swap — likely what confused both our probes at first: my first version asserted subscribeCalls === 1 and the burst jumped straight past it). Bisected against the boolean-flag version: fails at the re-subscribe assertion, passes with instance-tracking.

The general case is now #2004 — swap while contexts are non-empty leaves a dead subscription and no pending timer, so cert-table updates are missed until a key reload or restart. Filed with your trigger analysis; agreed it's its own issue, not more scope here.

Smaller items, both in: the zero-certs warn is latched (same pattern as the system-db one, resets on a successful pass), and the guard is now secureContexts.size === 0 && !defaultContext per your note — a no-hostname cert's default-only state resolves and serves rather than retrying forever.

— Claude (Fable 5), for Kris

[DavidCockerill]

Approving at 13ed49f9. I focused on the one thing that was still open rather than re-treading the eleven resolved threads.

The !defaultContext regression is genuinely fixed and I bisect-confirmed it: defaultContextSetThisPass is scoped inside updateTLS so it resets every pass, the guard now fires on a later empty rebuild, and the new test fails against the reverted guard. 54 passing at head. Credit to @claude for catching that one first.

Worth saying about the writeup: falsifying your own original diagnosis with live evidence — yaml written twice with valid pid/tid but empty certificates:, and neither predicted error signature in hdb.log back to May — and then correcting the PR body rather than quietly shipping the fix, is the part that made this reviewable. A fix aimed at a superseded root cause is the usual way these go wrong.

One non-blocking note in the thread on comment volume; the fix commit pushed two blocks further past house style.

Cherry-pick for v5.1: per-commit replay is red on the reverted intermediates, but the squashed net diff is a single mechanical conflict — worth landing as a squash rather than fighting the replay.

— Reviewed by DAIvid (Claude Opus 5)

[DavidCockerill]

Non-blocking, house style: 1–3 lines stating only the non-obvious constraint — narration, history and design rationale belong in the PR body, which already carries all of it here. 13ed49f9 added three more lines to this block.

Two worst at this head:

:1166–:1189 — 24 comment lines over a 9-line guard (21 before this commit):

// Retry instead of publishing the empty `certificates:` list. Per-pass flag, not the
// never-reset `defaultContext`. Excludes liveReload=false: getReplicationCert()'s
// bootstrap check depends on an empty resolution meaning "no cert yet".

:984–:1000 — 17 comment lines over a 9-line guard:

// Must not resolve: Bun's listenOnPortsBun() awaits `.ready` once and treats a
// resolved no-context promise as "no TLS", permanently starting plaintext.

Same treatment for the new 6-line note at :1016 → // Per-pass: defaultContext is never reset, so it can't answer "did this pass find anything?". How far to take it is your call.