NATS Library API Review and Enhancement

This report is a collection of best practices for NATS. want you to review the APIs that we expose in this library to see that they are simple and yet powerful capturing, uh, important use cases across services. I also want you to review how this can be used in your pipeline's entire API and entire db core. ensure that we don't break changes, and we actually provide a useful, um, logic.
The library read is complete and tests pass. Version/adoption picture so far: go-nuts is at v0.4.1; entire-api pins v0.4.0, mirror-pipeline pins v0.3.0, and entiredb doesn't import go-nuts at all yet (still on its internal copy that go-nuts was extracted from). I'm waiting on the three repo-mapping agents to report back with the detailed NATS usage in each consumer before writing up the full review.
The entire-api map is in — rich material: three consumers already migrated to jsconsumer, but seven more still hand-roll the exact scaffold go-nuts provides, plus three hand-rolled ack-heartbeat tickers (natsmsg.KeepInProgress exists for this) and an unused go-nuts/backoff despite three local backoff implementations. Still waiting on the entiredb and mirror-pipeline maps before I write the full review.
The mirror-pipeline map is in as well — it's the heaviest go-nuts consumer (13 Connect sites) but pinned to v0.3.0, which predates the jsconsumer/natsmsg/backoff subpackages entirely; its fanoutengine, otelnats, and worker heartbeat/backoff code are hand-rolled equivalents of exactly those. One report left (entiredb), then I'll write up the full review.
All verification is done — the review is complete. Here it is.
go-nuts API review — against NATS best practices and actual usage in entire-api, entiredb, and mirror-pipeline
A note first: I couldn't find the best-practices report you mentioned anywhere in the workspace — nothing matching it exists in any of the six repos. I grounded the review in canonical NATS/JetStream guidance (official docs + nats.go v1.52 source, which I verified directly where it mattered) instead. If you share the report, I can cross-check it point-by-point.
TL;DR: The library is in very good shape — small, orthogonal, correct on the subtle stuff (drain flush budgets, rotation-aware mTLS, capped ack-extension, Term-on-poison), and every released version is backward compatible: I diffed v0.3.0..HEAD on the root package and v0.4.0..v0.4.1 on everything — zero API or behavior changes, upgrades are purely additive. The real findings are about coverage: the three services still hand-roll five patterns the library should own, and two missing jsconsumer knobs are hard blockers for migrating entiredb's consumers.
What the library gets right
The design matches best practice almost everywhere it has an opinion: reconnect-forever with fail-fast initial dial (connect.go:108-124), Drain-not-Close with the publish-flush budget correctly accounted on top of nats.DrainTimeout (drain.go:40-50 — a subtlety most codebases get wrong), cancel→join→drain ordering in ShutdownGroup, durable-required + AckExplicit + bounded AckWait/MaxDeliver in jsconsumer, capped KeepInProgress so a wedged handler still redelivers, Term-on-undecodable so poison doesn't spin, and per-handshake cert/CA reload with hostname re-verification and TLS 1.3 minimum. The root-vs-subpackage dependency split is genuinely useful, and natsmsgtest.FakeMsg is exactly the right test seam. Tests pass on HEAD.
Gaps, ranked by impact
1. There is no publish-side story, and it's the biggest duplication across all three services. Every service has hand-rolled the same publish core — producer span + trace Inject + Nats-Msg-Id dedup header + bounded pub-ack wait (universally 5s): mirror-pipeline's internal/natspub (behind six typed publishers), three separate copies of a 5s publishTimeout in entire-api (forwarder/publisher.go:31, settingsrelay/relay.go:60, mirrorlifecycle/consumer.go:54), and three duplicated headerCarrier implementations inside entiredb publishers (repoops/publisher.go:134-152, gitjobs/publisher.go:133-151, resourceevents.go:282-300). A small natsmsg publish helper (span + inject + msg-id + timeout, on the modern jetstream API) plus a StartProducerSpan symmetric to StartConsumerSpan would capture the single most-repeated pattern in the fleet. Inject alone covers only a third of what publishers actually do.
2. jsconsumer.Config is missing knobs that are hard migration blockers. InactiveThreshold — required by every consumer on the interest-retention webhook stream (entiredb's permswebhook/consumer.go:57-73 and mirror-pipeline's forwarder both set 72h, and the forwarder even hand-rolls a reconcile via ConsumerInfo/UpdateConsumer because the legacy API lacked it) — cannot be expressed, so those consumers literally cannot migrate. MaxAckPending is also unexposed (matters for the shared-durable, multi-replica work queues). Both are additive one-field changes. Note MaxDeliver: -1 (unlimited, used by entiredb repoops) does pass through EffectiveMaxDeliver correctly — that one's fine.
3. jsconsumer has no supervision, and both non-adopters built one. entiredb's natsx.PullConsumer recreates the consumer with exponential backoff when the consume loop dies, and tolerates ErrStreamNotFound at boot as warn-and-retry (pullconsumer.go:56-139); mirror-pipeline's fanoutengine retries subscribe 1s→30s. go-nuts Start fails hard once and never re-establishes if the ConsumeContext closes permanently. A Run-style supervised variant (recreate-on-closed with backoff) is the third thing standing between entiredb and adoption.
4. backoff.Policy is flat-only, but two of three services use growing delays. entire-api naks with exponential 1s→300s (ingest/pipeline.go:305-323), entiredb repoops with geometric 5s→30s (repoops/consumer.go:796-809). The package doc argues flat-is-deliberate, but the fleet disagrees with the doc. An optional delay schedule (e.g. a DelayFor(numDelivered) hook or base/factor/cap fields, zero-value = current flat behavior) would let those consumers adopt without changing their semantics. Non-breaking.
5. Connect installs no async ErrorHandler — and nats.go's default prints to stderr. I verified in nats.go v1.52: with AsyncErrorCB nil, defaultErrHandler writes slow-consumer/permission-violation errors to raw stderr, bypassing the structured logger entirely. The proof this belongs in the library: mirror-pipeline attaches its own nats.ErrorHandler via WithNATSOptions at six Connect sites (worker, rekicker, admin, forwarder, fanout, natsregions) — permission violations on missing grants are exactly how their DLQ/advisory failures surface. A default handler routing through cfg.logger (overridable, since extra options apply last) closes it. Same argument, smaller stakes, for a LameDuckModeHandler log line so server-initiated evacuations are visible during maintenance.
Smaller notes: (a) a handler panic inside jsconsumer.Process crashes the whole process — the Consume callback runs on nats.go's dispatch goroutine, outside ShutdownGroup.Go's recover. mirror-pipeline's worker and forwarder both recover-and-Nak per message, and entiredb dead-letters on panic; if those migrate, that protection silently disappears unless Process gains an opt-in recover or the doc says handlers must recover. (b) The MAX_DELIVERIES advisory backstop (ChanQueueSubscribe + queue group + worker pool + GetMsg + idempotent record) is hand-rolled three times with the same shape (entire-api ingest/jobs.go:411-487, mirror-pipeline worker/advisory.go and webhookdlq/advisory.go) — a strong candidate for the next slice, along with a DLQ helper (entiredb's natsmsg.DeadLetter with its standard headers is the cleaner of the two existing shapes). (c) The README's root example uses the legacy sub.Fetch API while the subpackages are all on modern jetstream — worth a note in the example that it's the legacy path.
Fit per service
entire-api (v0.4.0) — best adopter, half-finished. Connect/Drain everywhere, three consumers on jsconsumer — but seven more still hand-roll the identical scaffold, and three hand-rolled InProgress tickers duplicate KeepInProgress. One migration caveat: their tickers fire at AckWait/2 with no cap; KeepInProgress fires at AckWait/3 capped at ~5×AckWait. For the analysis consumer (AckWait 60s → 5min cap) check LLM job durations before switching, or a legitimately-slow job gains a redelivery mid-run. Also cheap wins: the ingest fetch loop (pipeline.go:139-183) doesn't use IsShutdownFetchErr, and go-nuts/backoff is unused pending gap #4.
entiredb — zero adoption, which is ironic since the library was extracted from it: internal/natsx + internal/natsmsg are near-identical parallel implementations (its KeepInProgress is byte-for-byte the same semantics — that migration is a no-op). Blockers are exactly gaps #2 and #3 plus the DLQ helper. Two behavior deltas to handle deliberately at migration: entiredb connects with RetryOnFailedConnect(true) everywhere, while nuts.Connect defaults fail-fast — the go-nuts posture is arguably better ops hygiene, but it changes boot semantics, so either pass WithRetryOnFailedConnect() or consciously accept crash-loop-on-misconfig. And natsx.Drain installs a ClosedHandler (clobbering any existing one) where nuts.Drain correctly uses a status listener — adoption is a strict improvement there. Separate small bug it would fix: entire-server's ref-events publisher tears down with bare nc.Close() (ref_events_publisher.go:93), dropping buffered publishes on shutdown.
mirror-pipeline (v0.3.0) — heaviest root-package user (13 Connect sites, the only ShutdownGroup user), but pinned before the subpackages existed; its fanoutengine/otelnats/natspub/worker-heartbeat code is the donor material for them. Bumping to v0.4.1 is verified-safe. One live bug adoption would fix: meta-fanout's lifecycle consume loop (lifecycle.go:244-261) skips the IsShutdownFetchErr check, so every rollout drain logs a spurious "Lifecycle fetch error" — the exact COR-923 noise the classifier exists to suppress.
Recommended order
- Additive
jsconsumer.Configfields (InactiveThreshold,MaxAckPending) + a supervisedRunvariant — unblocks entiredb. - Publish helper in
natsmsg(producer span + inject + msg-id + bounded ack) — retires the most duplicated code in the fleet. - Default
ErrorHandler(+ lame-duck log) inConnect— one-file change, immediately improves observability at every existing call site on next bump. - Backoff schedule extension, then DLQ/advisory-backstop slice as the follow-up milestone.
All four are non-breaking; nothing in the current surface needs to change shape. The existing API is the right size — the risk here isn't over-design, it's that the missing 20% keeps entiredb and half of entire-api on forks of the same logic, which is the situation the library exists to end.
All four review recommendations are implemented, tested, and committed on branch nodo/go-nuts-extension — six commits, each with the checkpoint trailer, full suite green with the race detector, lint clean.
What landed:
-
connect.go—Connectnow installs a defaultnats.ErrorHandlerrouting async errors (slow-consumer drops, permissions violations) through the configuredsloglogger instead of nats.go's stderr default, plus a lame-duck warn log. Both are overridable viaWithNATSOptionssince extras apply after defaults. Tested against a real embedded server: a denied subscription's async permissions violation and a realLameDuckShutdownboth produce the expected log lines. -
jsconsumer—ConfiggainsInactiveThresholdandMaxAckPending(plain pass-throughs, zero-value compatible; test asserts they land on the on-server consumer), which unblocks the interest-stream webhook consumers in entiredb and mirror-pipeline. -
jsconsumer.Run— the supervised variant ofStart: recreates the consume loop with exponential backoff (1s→30s, reset after a healthy run) when it closes underneath the caller, treats a not-yet-provisioned stream at boot as retry-not-crash, fails fast only on config errors, and joins the live loop on cancel so it slots intoShutdownGroup.Go. Tests cover consumer-deleted-and-recreated, stream-appears-late, and clean cancel — all against a real broker. One design detail: each attempt gets a child context soStart's cancel-watcher goroutines don't accumulate across recreates on a flapping consumer. -
natsmsg.Publisher+StartProducerSpan— the publish core every service had copied: producer span, trace inject,Nats-Msg-Iddedup, bounded pub-ack wait (default 5s). Tests verify stored-message headers, broker-side dedup (PubAck.Duplicate), that an empty msgID doesn't clobber a caller-set header, and that an unbound subject errors promptly. -
backoff.Factor/MaxDelay+DelayFor— the growing envelope both entire-api (1s→300s) and entiredb (5s→30s) actually run, now expressible; zero value keeps flat behavior exactly, and uncapped growth saturates instead of wrapping negative. -
README and
doc.goupdated, including the note that the root usage example is the legacy pull API.
Everything is additive — no existing signature or default changed, so v0.3.0/v0.4.0 consumers upgrade cleanly. I've left the branch local; say the word and I'll push it and open a PR. The DLQ helper + advisory-backstop slice (and the handler-panic posture question in Process) remain as the follow-up milestone — I've noted those and the per-service migration caveats in memory.
PR created: https://github.com/entireio/go-nuts/pull/6
Branch pushed through the entire mirror, six commits, all checkpoint trailers intact. The PR body covers the five changes, the real-broker test coverage, and flags the follow-up milestone (DLQ helper + advisory backstop + Process panic posture) so it doesn't get lost.
Local main is synced with the merge (c8bf094) and the working branch is deleted.
One thing gates everything downstream: the services can't pin these changes until there's a release tag — the new surface is all additive, so by the repo's own versioning pattern that would be v0.5.0 on c8bf094. Want me to tag and push it? After that, the natural follow-ups in priority order are: bump mirror-pipeline from v0.3.0 (its meta-fanout lifecycle loop's spurious rollout errors and six hand-attached ErrorHandlers become deletable immediately), bump entire-api from v0.4.0, and start the entiredb migration that the new Run/InactiveThreshold/MaxAckPending surface just unblocked.
v0.5.0 is tagged and live — pushed through the mirror and confirmed on GitHub pointing at the merge commit (c8bf094), so go get github.com/entireio/go-nuts@v0.5.0 will resolve for all three services.
Ready when you are for the adoption work — mirror-pipeline's bump from v0.3.0 is the highest-value first move (it picks up the entire subpackage surface at once, fixes the spurious rollout errors in the meta-fanout lifecycle loop, and makes its six hand-attached ErrorHandlers redundant).
Understood — leaving entire/checkpoints/v1 untouched. It's flagged and you've made the call; no further action there.
That closes the public-readiness pass. Where go-nuts stands:
- v0.5.0 — tagged, pushed, resolves through the public Go proxy (so pkg.go.dev will index it).
main/v0.5.0tree is library-only, MIT detected. - No secrets anywhere in history.
- One cosmetic gap: the GitHub repo has no description set — worth a one-liner for a published library (e.g. "Connection-lifecycle and JetStream consumer helpers for NATS in Go"). Want me to set it?
gh repo edit entireio/go-nuts --description "..."— quick and reversible, but it's public-facing so I'll leave the call to you.
Otherwise the library is fully published and ready to consume. The pending follow-up whenever you want it: bumping the three services onto v0.5.0 — mirror-pipeline (from v0.3.0) is the highest-value first move, since it picks up the whole subpackage surface at once, fixes the spurious rollout errors in the meta-fanout lifecycle loop, and makes its six hand-attached ErrorHandlers redundant.
Done. Description set to "Connection-lifecycle and JetStream consumer helpers for NATS in Go" and confirmed live on the repo.
That wraps the publishing work — go-nuts is public, described, v0.5.0 tagged and resolving through the public proxy. Ready for the adoption bumps (mirror-pipeline first) whenever you want to pick that up.
Mostly yes — the prose is clear and the subpackage section is accurate (those are the parts I updated). But reading it as an outsider landing on a public repo, one real gap and a couple of smaller things stand out:
The opening tagline is now stale and undersells the library. Line 7 still says "Connection-lifecycle helpers for NATS pull consumers … for connecting, draining, and shutting down cleanly." That was true before the extension, but the library now also ships a full JetStream consumer scaffold, a publish helper, trace propagation, and a backoff policy — half its value. The GitHub description I just set is actually more accurate than the README's own first sentence. "What it provides" then only lists the three root-package items, so the subpackages read as an afterthought at the bottom rather than headline features.
Two internal-isms read differently on a public repo than they did on a private one:
- Line 116: the bare
(COR-762)is an internal Linear ticket ref — meaningless noise to an external reader. (There are more of these indoc.goand code comments, but the README is the front page.) - Lines 84–86:
Connectdefaults to readingENTIRE_INTERNAL_TLS_*env vars. It's honestly documented, but it signals "internal library that happens to be public." If go-nuts is meant as genuinely general-purpose, one line acknowledging it's opinionated toward Entire's conventions (overridable via Options) would set expectations; if it's really just public-because-convenient, that's fine too — but the current framing presents a clean general library while defaulting to org-internal behavior.
Minor: the usage snippet references sub and handle without showing where they come from, and never shows the import (module is …/go-nuts, package is nuts). Fine for an illustrative excerpt, but a newcomer might stumble.
The one I'd actually push on is the tagline — it's a genuine accuracy problem, not taste. Want me to rewrite the intro + "What it provides" so the JetStream/publish/backoff capabilities are represented up front, and drop the COR-762 ref? I can leave the env-var framing alone unless you want it addressed.
The README is fixed. Changes:
- New tagline now sets full scope — a toolkit for NATS and JetStream services, calling out the consumer scaffold, publish helper, trace propagation, and redelivery policy up front instead of only "connect/drain/shutdown."
- "What it provides" → "Connection lifecycle (root package)" so the three root items read as the lifecycle core and the
## Subpackagessection is clearly the rest, not an afterthought. The#subpackagesanchor in the usage note still resolves. (COR-762)dropped — no internal ticket refs remain in the README.
Left the env-var framing and the illustrative snippet as-is, per what we agreed.
The edit is on main in the working tree, uncommitted. Since you've been landing everything via PR (and main is likely protected), want me to branch → commit → PR this, or are you handling it? It's doc-only, so no new tag needed — though if you want pkg.go.dev to show the updated README, it refreshes when the next version tag is pushed.
PR opened: https://github.com/entireio/go-nuts/pull/7
Doc-only, one commit with the checkpoint trailer. No new tag needed — though pkg.go.dev will refresh the rendered README on the next version tag whenever you cut one.