feat: support JetStream API prefix

[joeriddles]

Support JetStream API prefix for rita.Manager

[synadia-claude-reviewer]

Claude finished @joeriddles's task in 3m 22s —— View job

---

Code Review

• Gather context and read changed files
• Analyze correctness and potential bugs
• Review NATS JetStream usage and API stability
• Post inline comments for specific issues
• Post overall review summary

---

Summary

The feature works for its stated purpose. Three issues worth addressing before merge:

1. Double-application of opts (manager.go lines 212–229) — real bug
The New function first runs all m.jsOpts manually to extract APIPrefix, then passes the same opts slice to jetstream.NewWithAPIPrefix, which runs them again internally. For the common case of simple field-setters this is harmless, but it violates the expectation that each opt fires exactly once and will break any opt that is not idempotent. The pre-run loop is unnecessary: jetstream.New(nc, m.jsOpts...) already applies opts to a JetStreamOptions struct and will pick up the APIPrefix field directly, making the NewWithAPIPrefix branch redundant. See inline comment at line 222.

2. Public API exposes nats.go internals — API stability risk
WithJetStreamOpts(jsOpts ...jetstream.JetStreamOpt) puts jetstream.JetStreamOpt on rita's public surface. For a pre-1.0 library this cements a coupling to nats.go's options mechanism — any rename or restructure there is a breaking change here. The primary stated need is a custom API prefix; a typed WithAPIPrefix(string) ManagerOption covers it cleanly. WithJetStreamOpts could stay as an escape hatch but shouldn't be the main path. See inline comment at line 63.

3. Replace-not-append semantics on a variadic function (minor)
WithJetStreamOpts replaces the opts slice rather than appending. Combined with the variadic signature this is unexpected — callers who call it twice lose the first set silently. Consider WithJetStreamOpt (singular, appends) or explicit documentation. See inline comment at line 65.

The test coverage in TestWithJetStreamOpts_APIPrefix is appropriate and NewNatsServerWithDomain is a clean addition to the test helpers.

[synadia-claude-reviewer]

Claude finished @joeriddles's task in 2m 22s —— View job

---

Code Review

• Read changed files
• Analyze correctness and potential bugs
• Review NATS JetStream usage and API stability
• Post inline comments
• Post overall review summary

---

Summary

The refactor commit (d3b1b59) cleanly addressed all three concerns from the previous review:

• Double-application of opts — gone; opts are now plain string setters, no JetStreamOpt intermediary
• Public API coupling to nats.go internals — resolved; WithAPIPrefix(string) exposes no nats.go types
• Replace-not-append semantics — moot; WithJetStreamOpts was removed entirely

The current implementation is correct and well-structured. Two minor issues remain:

1. apiPrefix field outlives its usefulness (manager.go:94)
The field is only used during New() to decide which constructor to call, then never read again. It can be a local variable inside New(), which avoids pinning a string allocation to the Manager for its entire lifetime. Not a bug, just slightly wasteful.

2. Silent no-op for WithAPIPrefix("") (manager.go:64)
An empty (or whitespace-only) string silently falls through to jetstream.New(nc) with no error. The behavior is arguably correct, but it should be documented so callers aren't surprised. A guard clause or a doc comment noting the no-op semantics would suffice.

The test coverage in TestWithAPIPrefix is solid — it spins up a real NATS server with a domain and verifies end-to-end that the prefix-routed JetStream can actually append and evolve events. NewNatsServerWithDomain is a clean, reusable helper.

[bruth]

A rebase is needed, but just to confirm, is this needed at the connection level? Or is do we need configuration at the event store level?

[joeriddles]

A rebase is needed, but just to confirm, is this needed at the connection level? Or is do we need configuration at the event store level?

The assumption this makes is all your event stores have the same JS prefix, so the connection level makes sense in my mind.

[synadia-claude-reviewer]

Claude finished @joeriddles's task in 1m 54s —— View job

---

Code Review

• Gather context
• Read changed files
• Analyze correctness and potential bugs
• Review NATS JetStream usage and API stability
• Post inline comments for specific issues
• Post overall review summary

---

Summary

The implementation is correct and the previous review's three concerns are fully resolved. One minor issue remains from the prior round:

WithAPIPrefix("") is a silent no-op (manager.go:64) — the doc comment says "sets a custom JetStream API prefix" but an empty string falls through to jetstream.New(nc) without any signal to the caller. This should either be validated (return an error) or documented explicitly. Inline comment left with a suggestion.

Everything else looks good:

• jetstream.NewWithAPIPrefix is used correctly with no double-application of opts
• The apiPrefix field on Manager is only written during construction and read once in New() — noted previously and dismissed as acceptable
• NewNatsServerWithDomain is consistent with the existing test helper pattern
• TestWithAPIPrefix correctly exercises the full path: server with domain → connect → create store → append → evolve

No correctness bugs, no security issues, no JetStream durability or ack concerns introduced.