[FIXED] Route missing after reconnect when duplicate resolution kept gossip URL

[MauriceVanVeen]

Follow-up of #8454

Similarly found in Antithesis, for a $SYS route not re-establishing. Turned out to be the same bug as before, in handleDuplicateRoute versus upgradeRouteToSolicited, for any duplicate pinned routes or normal duplicate routes in non-pool mode.

[synadia-claude-reviewer]

Claude finished @MauriceVanVeen's task in 2m 3s —— View job

---

PR Review: Route missing after reconnect (duplicate resolution + gossip URL)

• Gather context and read PR changes
• Analyze handleDuplicateRoute and upgradeRouteToSolicited changes
• Review test coverage
• Post findings

---

Summary

This is a targeted, correct fix. The bug is symmetric with the one fixed in #8454: upgradeRouteToSolicited got the URL-promotion logic there, and this PR applies the same logic to handleDuplicateRoute, which is the other code path for pinned (per-account) duplicate routes and non-pool mode duplicate routes.

Root Cause

When handleDuplicateRoute was called with an incoming c that was Explicit (configured URL) against a surviving remote that had already didSolicit = true (but with a gossip/Implicit URL), the old guard:

if didSolicit && !remote.route.didSolicit {   // false — remote was already solicited
    remote.route.url = url                     // SKIPPED: gossip URL stays
}

...prevented the URL upgrade. On disconnect, client.go:6378 captures rurl = c.route.url (the gossip URL), and routeStillValid rejects it since it doesn't match any entry in opts.Routes, silently abandoning the reconnect.

Fix Analysis

The new condition mirrors upgradeRouteToSolicited exactly:

// upgradeRouteToSolicited (existing, from #8454)
if !r.route.didSolicit || (rtype == Explicit && r.route.routeType != Explicit) {
    r.route.url = url
}

// handleDuplicateRoute (this PR)
if !remote.route.didSolicit || (rtype == Explicit && remote.route.routeType != Explicit) {
    remote.route.url = url
}

The symmetry is correct. The URL is promoted to the configured one only when: (a) the remote wasn't yet soliciting, or (b) the incoming is Explicit and the remote isn't yet Explicit. In all other cases (both already Explicit, or both Implicit) the URL is left unchanged, which is safe.

Concurrency: c.mu is acquired and released before remote.mu is taken — no double-lock, no deadlock risk. The fields read from c.route are properly snapshotted before the lock is dropped.

Test Coverage

The new TestRouteReconnectAfterDuplicateRouteAdoptsConfiguredURL test directly exercises the bug path:

1. Sets a pinned route's state to didSolicit=true, routeType=Implicit, url=gossipURL
2. Calls handleDuplicateRoute with an Explicit dup
3. Asserts the URL was promoted to the configured one
4. Closes the connection and verifies the cluster re-forms
5. Verifies interest flows across the re-established pinned route

The test is well-structured. One minor note: the setup mutates pinned.route fields without holding pinned.mu, but since checkClusterFormed has returned and the cluster is quiesced, no concurrent route writers should be active — acceptable for test code.

The rename of the existing test (TestRouteReconnectAfterDuplicateRouteUpgrade → TestRouteReconnectAfterSolicitedUpgradeAdoptsConfiguredURL) is appropriate since it now clearly distinguishes the two paths under test.

Verdict

LGTM. No correctness, security, or performance concerns. The fix is minimal, symmetric, and well-tested.

[neilalexander]

LGTM