(2.15) [FIXED] Leafnode JetStream isolation/extension with same domain & sys account

[MauriceVanVeen]

The extend/isolate decision for leafnode connections binding the system account only compared configuration claims (same JetStream domain + shared system account), without checking whether either side actually has a meta controller to extend. A standalone JetStream server (no cluster/gateway, or a solicited side without extension_hint: will_extend) never starts one, yet the extend branch was still taken. Resulting misbehavior:

• Two independent JetStreams under the same domain kept exchanging system account JS API/raft traffic ($JS.API.>, $NRG.>, $JSC.>) across the leafnode connection.
• A leaf running a meta group for extension, soliciting a standalone hub, waited in observer mode forever, leaving its JetStream unusable. However, a standalone leaf with an explicit will_extend will remain in observer mode until the leaf/hub is properly configured.

Servers now advertise whether they can take part in a shared meta group (js_no_ext in the leafnode INFO and CONNECT), derived from configuration and runtime state (since remotes can be config reloaded to add/remove the system account connection and must then not return to bad behavior). If either side can't extend, the connection is isolated with the same denies as a domain mismatch, on both ends independently. Old servers never set the field, so mixed-version links keep their existing behavior.

Additionally, log messages are improved: leaf extension now warns when locally created streams/consumers are orphaned and deleted as a result of adopting the parent's meta state, and isolation/extension log messages are clarified.

Intended configurations are the following:

• No extension: separate JetStream domains, system accounts are allowed to be shared for monitoring purposes, etc.
• With extension: both the JetStream domain and system account are shared and the hub is clustered.

Other permutations are deemed as misconfigurations, and the server will now properly isolate in these cases.

[synadia-claude-reviewer]

Claude encountered an error after 7m 55s —— View job

---

PR Review

• Gather context and understand the PR
• Read PR diff and changed files
• Review leafnode.go changes (core logic)
• Review jetstream.go changes
• Review server.go changes
• Review test coverage
• Post inline comments and final review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 07637f799f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[MauriceVanVeen]

@claude, review.

[synadia-claude-reviewer]

Claude finished @MauriceVanVeen's task in 5m 30s —— View job

---

PR Review

• Gather context and understand the PR
• Read PR diff and changed files
• Review leafnode.go changes (core logic)
• Review jetstream.go changes
• Review server.go and reload.go changes
• Review test coverage
• Post final review

---

Overview

This is a well-motivated fix for a real correctness bug. The core insight is sound: the old addLeafNodeConnection decided isolation/extension purely from configuration claims (same domain + shared sys account), without asking whether either side actually has a meta controller to extend. The new protocol flag (js_no_ext in both INFO and CONNECT) plus the runtime check (jetStreamStartedWithoutMetaController) close both the config-only gap and the reload gap cleanly.

---

What looks good

Protocol design: Advertising js_no_ext on both the INFO (hub→leaf) and the CONNECT echo (leaf→hub) means each side independently determines isolation. Old servers never set the field, so mixed-version deployments fall back to the existing behavior. The field is omitempty so it's wire-invisible when false.

Config + runtime split: The separation between jetStreamNotExtendable() (config-derived) and jetStreamStartedWithoutMetaController() (runtime) is clean and correctly models the two distinct failure modes: wrong config vs. "started before the sys remote existed".

Reload path: updateLeafNodeJSNoExtInfo() is called from leafNodeOption.Apply() in reload.go and correctly regenerates leafNodeInfoJSON when extendability changes. The Codex reviewer's inline comment claiming this is missing appears to be a false positive — the function does update s.leafNodeInfoJSON through generateLeafNodeInfoJSON().

Observer mode for stuck will_extend: The new path that keeps a standalone server in observer mode (rather than forming its own single-peer meta group) when the hub can't be extended is the right behavior.

Orphan warning: The new warning about locally-created streams/consumers being deleted when the extension decision flips is genuinely useful for operators:

// leafnode.go:2155
c.Warnf("Extending JetStream domain %q orphans local JetStream state: %d stream(s) and %d consumer(s)...", ...)

Test coverage: The TestJetStreamLeafNodeDomainSysAccountPermutations matrix covers all meaningful permutations (8 sub-tests), and the two standalone reload tests are exactly what's needed to validate the updateLeafNodeJSNoExtInfo path. The checkSysLeafDenied helper correctly verifies that both pub and sub denies are set on the sys account connection.

---

Findings

1. Existing connections are not re-evaluated after a reload changes extendability

updateLeafNodeJSNoExtInfo() correctly updates leafNodeInfoJSON for new connections, but does not call sendAsyncLeafNodeInfo(). Even if it did, the isolation logic in addLeafNodeConnection only runs once per connection — an already-established leaf connection would not be re-isolated.

For the "sys remote removed by reload" case this is generally safe because removing the remote closes the existing sys account connection, which triggers a reconnect where the new INFO is applied. For "sys remote added by reload", the reverse: the new sys account connection runs addLeafNodeConnection fresh and will correctly see localNoExt = true (started without meta controller).

The test TestJetStreamLeafNodeSysRemoteRemovedByReloadNotExtendable creates the bottom leaf after the reload, so it doesn't cover an already-connected bottom leaf. This is a known gap but may be acceptable given the reconnect behavior.

2. Warn message fires on every reconnect for the will_extend-stuck-in-observer path

// leafnode.go:2106-2107
c.Warnf("JetStream can not be extended over this connection and a single standalone server can not form its own meta group, JetStream will be unavailable")
c.Warnf(`Remove the JetStream Option "extension_hint: %s" and restart to run JetStream standalone instead`, jsWillExtend)

These warnings fire inside addLeafNodeConnection with no throttle. If the leaf reconnects frequently (which it will, since it's in a broken config), operators will see these repeated for every reconnect attempt. The existing codebase does similar things elsewhere, so this may be acceptable style, but it's worth noting.

3. Log message is slightly misleading when BOTH sides are standalone

When both hub and leaf are standalone with the same domain and shared sys account (the standalone-hub-standalone-leaf-no-hint case), both sides enter if localNoExt and log:

JetStream not extended, this server runs standalone JetStream and can not take part in a JetStream meta group

Neither side mentions the other is also standalone. Cosmetic only.

4. localNoExt vs myRemoteNoExt precedence in log message

// leafnode.go:2083-2093
if opts.JetStreamDomain == myRemoteDomain && (myRemoteNoExt || localNoExt) {
    if localNoExt {
        ...  // only local side mentioned
    } else {
        c.Noticef("JetStream not extended, remote server runs standalone JetStream and can not be extended")
    }

When both myRemoteNoExt and localNoExt are true, only the local-side message is emitted. In practice this is unlikely (both sides would have to report standalone simultaneously), but the condition (myRemoteNoExt || localNoExt) entering the block doesn't match the inner if localNoExt being exhaustive.

5. blockMappingOutgoing skip when domains match — could use a test

// leafnode.go:2206
if blockMappingOutgoing && opts.JetStreamDomain != myRemoteDomain {

The comment explains the reasoning (remote has identical domain mappings; a deny would cause permission violations that close the connection). The existing permutation tests implicitly validate this, but there's no dedicated test asserting the connection does not get closed in the same-domain isolated case. This is a subtle invariant that would be nice to have pinned.

6. jetStreamNotExtendable() returns true for standalone+no-sys-account case even when JetStream is disabled — actually returns false early via !opts.JetStream, so this is fine. Confirmed correct.

---

On the Codex inline comment

The Codex comment says leafNodeInfoJSON stays stale after a reload. This is addressed: updateLeafNodeJSNoExtInfo() → generateLeafNodeInfoJSON() regenerates the JSON, and it's wired into leafNodeOption.Apply() in reload.go. The stale-for-existing-connections concern (point 1 above) is a separate, narrower issue.

---

Summary

The fix is correct and the test coverage is solid for the primary failure modes. The main gaps are (a) already-connected leaves not being re-isolated on reload and (b) warn message spam on reconnects in broken configs. Neither blocks merging — they're known limitations with the current architecture. The protocol flag design, the runtime detection, and the chained-leaf test case are all well-handled.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d5eee3a765

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".