(2.15) [ADDED] Domain prefixed JetStream API in the system account

[MauriceVanVeen]

The system account can be shared between a hub and leaf cluster, but having different domains. This ensures they are treated as two separate JetStream meta groups. Before this PR it wasn't possible to administer the leaf cluster from the hub, since the system account's JS API wasn't domain-aware, and the servers added automatic pub/sub denies to $JS.API.> so JetStreams of different domains don't cross-talk about a system JS API request. This PR preserves the pub/sub denies and adds support for the domain-aware JS API, by allowing the same $JS.<domain>.API.> convention that accounts use.

[synadia-claude-reviewer]

Claude finished @MauriceVanVeen's task in 10m 24s —— View job

---

PR Review: (2.15) [ADDED] Domain prefixed JetStream API in the system account

• Gather context and read CLAUDE.md
• Analyze PR diff
• Review server/leafnode.go changes
• Review server/jetstream.go changes
• Review test coverage
• Post inline comments and summary

---

Overall Assessment

The design of this PR is sound: adding $JS.<domain>.API.> → $JS.API.> to the system account's mappings, and extending the outgoing publish-deny to the system account in the isolated-leaf case, is the correct approach for cross-domain JS API administration.

One correctness issue was found — see the inline comment on jetstream.go.

---

Critical: Domain mapping lost after system-account JWT update

checkJetStreamExports only checks for the jsAllAPI service export. When the system account JWT is refreshed, updateAccountClaimsWithRefresh removes any mapping whose src is absent from the JWT — including $JS.<domain>.API.>, which is server-internal and will never appear in a JWT. After the removal, addSystemAccountExports → checkJetStreamExports is called, but since the service export still exists (se != nil), setupJetStreamExports is not called and the domain mapping is not re-added. The domain-prefixed API silently breaks until restart.

TestJWTSystemAccountJetStreamDomainMapping is explicitly written to catch this (its comment says "Make sure updating the system account does not drop it"), but checkJetStreamExports is never updated to check for the mapping — the test should be failing.

The fix is to also check whether the domain mapping is present in checkJetStreamExports and call setupJetStreamExports when it is missing. See the inline comment for a concrete diff. AddServiceExport is idempotent, so calling setupJetStreamExports when only the mapping is absent is safe.

---

leafnode.go change is correct

The key change is moving && acc != sysAcc out of the outer condition so the blockMappingOutgoing publish-deny is now also applied to the system account. In the isolated-leaf scenario (domains differ), the system account already receives denyAllJs on both pub/sub (covering $JS.API.>), but $JS.<domain>.API.> was previously unguarded. The new deny closes that gap. In the extending case (blockMappingOutgoing = false, same domain), the new code path is reached but the inner branch is a no-op — no regression there.

---

Minor observations

• s.Errorf vs s.Warnf: Using Errorf for the mapping failure and Warnf for the service export failure is slightly inconsistent; the mapping failure is arguably as serious. Not blocking.
• Tests TestJetStreamClusterSystemAccountDomainAPIIsolated and TestJetStreamClusterSystemAccountDomainAPIExtended provide solid end-to-end coverage of the two primary use cases.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3b5b37c6e1

ℹ️ 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".

[synadia-claude-reviewer]

Potential bug: domain mapping silently dropped on system-account JWT update.

When a JWT update arrives for the system account, updateAccountClaimsWithRefresh reconciles mappings: any m.src not in the JWT's ac.Mappings is removed — including $JS.<domain>.API.>, which is server-internal and never present in the JWT.

After removal, addSystemAccountExports → checkJetStreamExports is called, but checkJetStreamExports only checks for the jsAllAPI service export. Since that export still exists (se != nil), setupJetStreamExports is not called and the domain mapping is never re-added. The domain-prefixed API silently breaks until server restart.

TestJWTSystemAccountJetStreamDomainMapping is designed to catch exactly this — it pushes a new JWT and asserts the mapping survives — but it should be failing with the current implementation.

checkJetStreamExports needs to also verify the domain mapping is present:

func (s *Server) checkJetStreamExports() {
    if sacc := s.SystemAccount(); sacc != nil {
        sacc.mu.RLock()
        se := sacc.getServiceExport(jsAllAPI)
        needsDomainMapping := false
        if domain := s.getOpts().JetStreamDomain; domain != _EMPTY_ {
            domainAPI := fmt.Sprintf(jsDomainAPI, domain)
            needsDomainMapping = true
            for _, m := range sacc.mappings {
                if m.src == domainAPI {
                    needsDomainMapping = false
                    break
                }
            }
        }
        sacc.mu.RUnlock()
        if se == nil || needsDomainMapping {
            s.setupJetStreamExports()
        }
    }
}

AddServiceExport is idempotent (no error when re-adding an existing export), so calling setupJetStreamExports when only the mapping is missing is safe.

Fix this →

[MauriceVanVeen]

updateAccountClaimsWithRefresh already resets the exports before reconciling mappings. So checkJetStreamExports is guaranteed to find se == nil, to call setupJetStreamExports, and set up both the JS API service export and the domain mapping.

the test should be failing

TestJWTSystemAccountJetStreamDomainMapping passes today, and would only fail if the above invariant were broken.

[MauriceVanVeen]

@claude, brief re-review.

[synadia-claude-reviewer]

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

---

Brief Re-review

• Gather context
• Verify claim: updateAccountClaimsWithRefresh resets exports before reconciling mappings
• Read checkJetStreamExports / setupJetStreamExports
• Post re-review

---

Re-review findings

MauriceVanVeen's responses are correct on both points.

On the "critical" JWT update bug (now confirmed: not a bug)

The previous review incorrectly assumed the service export would still be present (se != nil) after a JWT refresh. Tracing the actual code path in updateAccountClaimsWithRefresh (accounts.go:3390):

1. Line 3440: a.exports = exportMap{} — the entire export map is reset to empty.
2. Lines 3482–3506: The domain mapping ($JS.<domain>.API.>) is added to removeList (absent from JWT) and subsequently removed.
3. Lines 3509–3510: addSystemAccountExports(a) is called, which calls checkJetStreamExports.
4. In checkJetStreamExports, sacc.getServiceExport(jsAllAPI) returns nil — because the export map was cleared in step 1.
5. se == nil is therefore always true in this path → setupJetStreamExports() is called → both the service export and the domain mapping are re-added.

TestJWTSystemAccountJetStreamDomainMapping correctly exercises this: it pushes a new JWT with an unrelated mapping, then asserts the domain mapping survives — and it does pass. The previous review comment was a false positive.

On the Codex P2 (getOpts().JetStreamDomain vs js.config.Domain): Deferring as pre-existing is appropriate.

The leafnode.go change is correct — moving && acc != sysAcc into the inner block means blockMappingOutgoing applies to the system account in the isolated-leaf case, closing the gap for $JS.<domain>.API.>.

This PR looks good to merge.