[fix] Re-probe reachability while the relay is firewalled

.claude/lessons.md

@@ -41,6 +41,13 @@ port.** The startup log therefore reads as though `--port` was ignored, which se
 debugging down the wrong path entirely. Log the *configured* UDP port explicitly at
 startup, and read a port mismatch as "firewalled" before reading it as "misconfigured".
 
+**dht-rpc probes reachability once, and never again while the public host is unchanged.**
+Its periodic re-check is guarded by `_lastHost === _nat.host` ("do not recheck the same
+network"), and the probe needs 3 of 5 ping-backs, so one unlucky round at startup left a
+correctly forwarded relay firewalled for hours. The tell: firewalled from the first seconds
+of a run, settings untouched, and a plain restart fixes it. `src/reprobe.js` re-runs
+`_updateNetworkState()` while firewalled; re-check that hook on any hyperdht bump.
+
 **On Docker Desktop, `firewalled: true` and a 503 from the probe are the expected
 result.** The VM's NAT means the relay is genuinely not publicly reachable, so a local
 Docker run smoke-tests boot and serve — never reachability. Don't debug it as a failure.

OPERATIONS.md

@@ -79,6 +79,15 @@ curl -s localhost:9200/readyz
 `firewalled: true` → clients cannot reach you. Check, in order: the host firewall,
 the cloud security group, and whether the UDP port is actually forwarded.
 
+The verdict can also simply be wrong for a while. HyperDHT probes reachability once
+at startup — it needs 3 of 5 remote nodes to ping the port back — and on its own
+never probes again while the public address is unchanged, so one lost round used to
+leave a correctly forwarded relay firewalled until the next restart. The relay now
+re-runs the probe itself while firewalled: after 1 minute, then backing off to every
+15. A relay that flips to reachable minutes after a restart, with
+`reachability re-probe passed` in the log, was that case. It does not apply under
+`MIRALL_RELAY_ASSUME_REACHABLE`, where nothing was probed to begin with.
+
 Two states the page separates that `/readyz` does not:
 
 - **Assumed reachable.** `MIRALL_RELAY_ASSUME_REACHABLE` makes `firewalled` read

src/relay.js

@@ -22,6 +22,7 @@ import Relay from 'blind-relay'
 import b4a from 'b4a'
 import { bootstrapNodes } from './config.js'
 import { resolveSeed, keyPairFromSeed, publicKeyZ32 } from './keys.js'
+import { startReprobe } from './reprobe.js'
 
 // udx's socket.address() returns { host, family, port } — NOT Node's dgram shape
 // with `address`. Normalised here so the status snapshot has one contract and a
@@ -47,6 +48,7 @@ export class RelayNode {
     this.dht = null
     this.server = null // hyperdht server (our listening identity)
     this.relay = null // the single blind-relay.Server
+    this.reprobe = null
     this.keyPair = null
     this.ready = false
     this.closing = false
@@ -158,6 +160,17 @@ export class RelayNode {
       )
     }
 
+    // Asserted reachability was never probed, so there is nothing to re-run.
+    if (!cfg.assumeReachable) {
+      this.reprobe = startReprobe(this.dht, {
+        onResult: (firewalled) => {
+          this.metrics?.m.dhtFirewalled.set(firewalled ? 1 : 0)
+          if (!firewalled) logger?.info({ publicKey: this.publicKeyZ32 }, 'reachability re-probe passed — the relay is now reachable')
+        },
+        onUnsupported: () => logger?.warn('this dht-rpc has no re-probe hook — a firewalled verdict will stand until restart')
+      })
+    }
+
     logger?.info({
       publicKey: this.publicKeyZ32,
       firewalled: this.firewalled,
@@ -266,6 +279,7 @@ export class RelayNode {
     this.ready = false
     this.metrics?.m.ready.set(0)
 
+    this.reprobe?.stop()
     this.meter?.stop()
     this.firewall?.stop()
 

src/reprobe.js

@@ -0,0 +1,57 @@
+// Re-runs dht-rpc's reachability probe while the node is firewalled.
+//
+// WHY IT EXISTS: dht-rpc probes once at bootstrap, and its periodic re-check is
+// skipped while the observed public host is unchanged ("do not recheck the same
+// network"). The probe needs 3 of 5 remote nodes to ping back within a timeout,
+// so one unlucky round — common in the first seconds after a restart — left a
+// correctly forwarded relay firewalled until the next restart, hours later.
+//
+// _updateNetworkState is dht-rpc internals: the same call its own tick makes,
+// minus the same-host guard. Everything about it is optional, so a dht-rpc that
+// drops it costs the self-heal and nothing else.
+const INTERVAL_MS = 60_000
+const MAX_INTERVAL_MS = 15 * 60_000
+
+export function startReprobe (dht, {
+  onResult = null,
+  onUnsupported = null,
+  intervalMs = INTERVAL_MS,
+  maxIntervalMs = MAX_INTERVAL_MS,
+  setTimer = setTimeout,
+  clearTimer = clearTimeout
+} = {}) {
+  let timer = null
+  let stopped = false
+  let wait = intervalMs
+
+  function schedule () {
+    if (stopped || !dht.firewalled) return
+    timer = setTimer(probe, wait)
+    timer.unref?.()
+    wait = Math.min(wait * 2, maxIntervalMs)
+  }
+
+  async function probe () {
+    timer = null
+    try {
+      await dht._updateNetworkState()
+    } catch { /* a failed probe is just another firewalled result */ }
+    if (stopped) return
+    onResult?.(!!dht.firewalled)
+    schedule()
+  }
+
+  if (typeof dht._updateNetworkState !== 'function') {
+    if (dht.firewalled) onUnsupported?.()
+  } else {
+    schedule()
+  }
+
+  return {
+    stop () {
+      stopped = true
+      if (timer) clearTimer(timer)
+      timer = null
+    }
+  }
+}

test/unit/reprobe.test.js

@@ -0,0 +1,92 @@
+// The reachability re-probe. Driven by a stand-in DHT and a hand-cranked timer,
+// so every branch runs without a network or a clock.
+import test from 'node:test'
+import assert from 'node:assert/strict'
+import { startReprobe } from '../../src/reprobe.js'
+
+function fakeTimers () {
+  const pending = []
+  return {
+    pending,
+    setTimer: (fn, ms) => { const t = { fn, ms, unref () {} }; pending.push(t); return t },
+    clearTimer: (t) => { const i = pending.indexOf(t); if (i !== -1) pending.splice(i, 1) },
+    async fire () { const t = pending.shift(); await t.fn(); return t.ms }
+  }
+}
+
+function fakeDht (passesOnProbe) {
+  const dht = {
+    firewalled: true,
+    probes: 0,
+    async _updateNetworkState () {
+      dht.probes++
+      if (dht.probes === passesOnProbe) dht.firewalled = false
+    }
+  }
+  return dht
+}
+
+test('a reachable node is never re-probed', () => {
+  const timers = fakeTimers()
+  const dht = { firewalled: false, _updateNetworkState: () => assert.fail('must not probe') }
+  startReprobe(dht, timers)
+  assert.equal(timers.pending.length, 0)
+})
+
+test('a firewalled node is re-probed until a probe passes, then left alone', async () => {
+  const timers = fakeTimers()
+  const dht = fakeDht(3)
+  const results = []
+  startReprobe(dht, { ...timers, onResult: (firewalled) => results.push(firewalled) })
+
+  await timers.fire()
+  await timers.fire()
+  assert.equal(dht.firewalled, true)
+  await timers.fire()
+  assert.equal(dht.firewalled, false)
+  assert.equal(dht.probes, 3)
+  assert.equal(timers.pending.length, 0, 'nothing is scheduled once it is reachable')
+  assert.deepEqual(results, [true, true, false])
+})
+
+test('the wait doubles up to a ceiling, so a truly closed port is not hammered', async () => {
+  const timers = fakeTimers()
+  startReprobe(fakeDht(Infinity), { ...timers, intervalMs: 1000, maxIntervalMs: 5000 })
+  const waits = []
+  for (let i = 0; i < 5; i++) waits.push(await timers.fire())
+  assert.deepEqual(waits, [1000, 2000, 4000, 5000, 5000])
+})
+
+test('a probe that throws does not end the retries', async () => {
+  const timers = fakeTimers()
+  const dht = { firewalled: true, _updateNetworkState: async () => { throw new Error('socket closed') } }
+  startReprobe(dht, timers)
+  await timers.fire()
+  assert.equal(timers.pending.length, 1)
+})
+
+test('stop cancels the pending probe, and one in flight schedules nothing after it', async () => {
+  const timers = fakeTimers()
+  const reprobe = startReprobe(fakeDht(Infinity), timers)
+  reprobe.stop()
+  assert.equal(timers.pending.length, 0)
+
+  const late = fakeTimers()
+  const dht = fakeDht(Infinity)
+  const running = startReprobe(dht, late)
+  const probe = late.fire()
+  running.stop()
+  await probe
+  assert.equal(late.pending.length, 0)
+})
+
+test('a dht-rpc without the private hook degrades to doing nothing', () => {
+  // The hook is dht-rpc internals. If a bump removes it the relay must still
+  // start; it just loses the self-heal, and says so once.
+  const timers = fakeTimers()
+  let warned = 0
+  const reprobe = startReprobe({ firewalled: true }, { ...timers, onUnsupported: () => warned++ })
+  assert.equal(timers.pending.length, 0)
+  assert.equal(warned, 1)
+  reprobe.stop()
+})