Retire Legacy NATS JetStream Publishing Bridges

Claude Code·Fable 5·

nodo·3mo ago·2hr 10min·1 Checkpoint·9 file changes·+141/-527·184K tokens

The mirror-pipeline survey is in (that's the last slice in the migration order, so I'll bank it): pkg/otelnats is pure trace propagation with 7 call sites, internal/natspub is a legacy-JetStream publish core used by 7 typed publishers — both map cleanly onto natsmsg — while two hand-rolled publishers (webhookdlq, lifecycle fan) and the worker/fanout fetch loops have semantics that stay local. It also flagged that mirror-pipeline vendors go-nuts, so the v0.8.0 bump needs a re-vendor there.

Still waiting on the entiredb and entire-api surveys before starting the first slice.

The entire-api survey is in. Key takeaways for that slice: all four target consumers are hand-rolled but already on the modern jetstream API with identical envelopes (AckWait 30s, MaxDeliver 8, plain Nak, Term on undecodable, no DLQ), and the repo already has five consumers on jsconsumer to use as the template. The forwarder also has the only publisher, with a hand-rolled producer span and dedupe-ID logic that maps onto natsmsg.Publisher with a caller-selected operation name. entire-api pins v0.7.0, so the bump to v0.8.0 also touches ingest/analysis/reattribute compile surfaces (out of scope to migrate, but must keep compiling).

One survey left — entiredb, which is the first migration slice. I'll start planning that PR the moment it lands.

medium effort → 3+5 angles × 6 candidates → 1-vote verify → ≤8 findings

You are reviewing for precision at medium effort: every finding you surface should be one a maintainer would act on.

Phase 0 — Gather the diff

Run git diff @{upstream}...HEAD (or git diff main...HEAD / git diff HEAD~1 if there's no upstream) to get the unified diff under review. If there are uncommitted changes, or the range diff is empty, also run git diff HEAD and include the working-tree changes in scope — the review often runs before the commit. If a PR number, branch name, or file path was passed as an argument, review that target instead. Treat this diff as the review scope.

Phase 1 — Find candidates (3 correctness angles + 3 cleanup angles + 1 altitude angle + 1 conventions angle, up to 6 each)

Run 8 independent finder angles via the Agent tool. Each surfaces up to 6 candidate findings with file, line, a one-line summary, and a concrete failure_scenario.

Angle A — line-by-line diff scan

Read every hunk in the diff, line by line. Then Read the enclosing function for each hunk — bugs in unchanged lines of a touched function are in scope (the PR re-exposes or fails to fix them). For every line ask: what input, state, timing, or platform makes this line wrong? Look for inverted/wrong conditions, off-by-one, null/undefined deref, missing await, falsy-zero checks, wrong-variable copy-paste, error swallowed in catch, unescaped regex metachars.

Angle B — removed-behavior auditor

For every line the diff DELETES or replaces, name the invariant or behavior it enforced, then search the new code for where that invariant is re-established. If you can't find it, that's a candidate: a removed guard, a dropped error path, a narrowed validation, a deleted test that was covering a real case.

Angle C — cross-file tracer

For each function the diff changes, find its callers (Grep for the symbol) and check whether the change breaks any call site: a new precondition, a changed return shape, a new exception, a timing/ordering dependency. Also check callees: does a parallel change in the same PR make a call unsafe?

Reuse

The angles above hunt for bugs; this one and the next two hunt for cleanup in the changed code. Flag new code that re-implements something the codebase already has — Grep shared/utility modules and files adjacent to the change, and name the existing helper to call instead.

Simplification

Flag unnecessary complexity the diff adds: redundant or derivable state, copy-paste with slight variation, deep nesting, dead code left behind. Name the simpler form that does the same job.

Efficiency

Flag wasted work the diff introduces: redundant computation or repeated I/O, independent operations run sequentially, blocking work added to startup or hot paths. Also flag long-lived objects built from closures or captured environments — they keep the entire enclosing scope alive for the object's lifetime (a memory leak when that scope holds large values); prefer a class/struct that copies only the fields it needs. Name the cheaper alternative.

Altitude

Check that each change is implemented at the right depth, not as a fragile bandaid. Special cases layered on shared infrastructure are a sign the fix isn't deep enough — prefer generalizing the underlying mechanism over adding special cases.

Conventions (CLAUDE.md)

Find the CLAUDE.md files that govern the changed code: the user-level ~/.claude/CLAUDE.md, the repo-root CLAUDE.md, plus any CLAUDE.md or CLAUDE.local.md in a directory that is an ancestor of a changed file (a directory's CLAUDE.md only applies to files at or below it). Read each one that exists, then check the diff for clear violations of the rules they state.

Only flag a violation when you can quote the exact rule and the exact line that breaks it — no style preferences, no vague "spirit of the doc" inferences. In the finding, name the CLAUDE.md path and quote the rule so the report can cite it. If no CLAUDE.md applies, return nothing for this angle.

Cleanup, altitude, and conventions candidates use the same file/line/summary shape; in failure_scenario, state the concrete cost (what is duplicated, wasted, harder to maintain, or which CLAUDE.md rule is broken) instead of a crash. Correctness bugs always outrank cleanup, altitude, and conventions findings when the output cap forces a cut.

Pass every candidate with a nameable failure scenario through — finders that silently drop half-believed candidates bypass the verify step and are the dominant cause of misses.

Phase 2 — Verify (1-vote, 3-state)

Dedup candidates that point at the same line/mechanism, keeping the one with the most concrete failure scenario. For each remaining candidate, run one verifier via the Agent tool: give it the diff, the relevant file(s), and the candidate, and have it return exactly one of:

CONFIRMED — can name the inputs/state that trigger it and the wrong output or crash. Quote the line.
PLAUSIBLE — mechanism is real, trigger is uncertain (timing, env, config). State what would confirm it.
REFUTED — factually wrong (code doesn't say that) or guarded elsewhere. Quote the line that proves it.

Keep candidates where the vote is CONFIRMED or PLAUSIBLE.

Output

Return findings as a JSON array of at most 8 objects:

Ranked most-severe first. If more than 8 survive, keep the 8 most severe. If nothing survives verification, return [].

3mo ago·11m

Verification is running on both repos now:

entiredb: review-pass fixes applied (shared LegacyJetStream interface everywhere, two more deliveryCount copies replaced with backoff.IsFinalDelivery, stale comments fixed) — lint + race re-running before I commit and push the update to PR #2793.
entire-api: all four consumers (githubpr, githubmeta, userlifecycle, forwarder) are migrated onto jsconsumer with contracts preserved (durables, filters, AckWait 30s, MaxDeliver 8, span names, metrics, logs); the forwarder publisher deliberately stays local since its span is a routing span with drop-reason semantics and it already uses the shared natsmsg.Inject. Unit + race tests pass; multicellular (embedded NATS) suite and lint are running.

I'll pick up when the background runs report.

COR-985 is implemented end to end: all three services are migrated onto go-nuts v0.8.0 in three independent, fully verified PRs, and the Linear issue is In Progress with everything linked.

The PRs

entiredb#2793 — repoops and permswebhook's duplicated geometric nak-backoff is replaced by backoff.Policy{Factor: 2} (tests now pin the exact 5s→10s→20s→30s-cap sequence so it can't drift); five legacy publishers (repoops, gitjobs, resync, permssweep incl. chunk continuations, permsbackfill) go through natsmsg.LegacyPublisher; stale natsx references are gone. A self-review pass with parallel finder/verifier agents then caught more dedup the migration had exposed: permssweep/permsbackfill's remaining deliveryCount copies became backoff.IsFinalDelivery, and the four now-identical publisher interfaces became natsmsg.LegacyJetStream.
entire-api#328 — githubpr, githubmeta, userlifecycle, and forwarder consumers move onto jsconsumer.Start/Process, one commit per slice in the ticket's order. Its review pass found one real regression — dropped jurisdiction/cluster fields on two startup log lines — which I fixed and pushed.
mirror-pipeline#221 — pkg/otelnats is deleted (natsmsg.Inject/Extract are drop-ins, and multi-value W3C baggage now propagates instead of truncating), and internal/natspub is reduced to domain-local concerns with its publish core on natsmsg.LegacyPublisher. Re-vendored, which is most of the diff by line count.

Deliberately not migrated (reasoning in each PR): entiredb's email-outbox/refevents/resourceevents publishers (no duplicated prologue to converge), entire-api's forwarder publisher (routing span with drop-reason semantics; its narrow modern-API seam is what tests fake), ingest/reattribute/analysis (ticket scopes them out), and mirror-pipeline's webhookdlq/lifecycle publishers plus the worker/fanout fetch loops (bespoke semantics the seam shouldn't own).

Verification — in each repo: embedded-NATS suites (entiredb's teardown/inactive-threshold/permssweep e2e; entire-api's multicellular cross-cell suite; mirror-pipeline's full test:race), go test -race, lint, go mod tidy -diff, and git diff --check, all with unmasked exit codes after I caught one gofmt failure hiding behind a pipe. Each PR documents its deliberate telemetry-plumbing deltas (bounded PubAck waits, no producer span on marshal failure, span status text, success-log span attribution, the empty-msg-ID edge).

Remaining before the ticket can close: the three PRs need review and merge. The ticket's own follow-ups (entire-api reattribute + analysis, with their heartbeat/long-handler behavior) stay separate as specified.

All three PRs confirmed merged, and I've moved COR-985 to Done in Linear (all three PRs are attached to the issue). Phase 3 is complete: entiredb, entire-api, and mirror-pipeline all run on go-nuts v0.8.0, with the duplicate delay calculations, publish prologues, and otelnats plumbing gone.

Two follow-ups the ticket names but that aren't tracked as issues yet, if you want them filed:

entire-api reattribute + analysis — the deferred long-handler consumers. Worth noting that jsconsumer.Config.KeepInProgress already exists for exactly their heartbeat pattern, so they may be a straightforward Phase 3.5.
go-nuts legacy-bridge deletion — LegacyPublisher/LegacyJetStream and the backoff *Legacy entry points are documented as transitional; once mirror-pipeline's remaining legacy nats.JetStreamContext callers (fanoutengine, worker) eventually move to the modern API, the bridges can be removed.

Happy to draft either ticket if useful.

Yes — in all three services, but the three bridge surfaces have very different usage profiles:

natsmsg.LegacyPublisher (the publish bridge) — 6 production call sites:

entiredb: the five publishers this migration converted — repoops, gitjobs, resync, permssweep, permsbackfill
mirror-pipeline: internal/natspub, behind all seven typed job publishers

natsmsg.LegacyJetStream via the DLQPublisher alias (the dead-letter publish surface) — this one is baked into the shared API itself: natsmsg.DeadLetter takes a DLQPublisher, which is LegacyJetStream. Users:

entiredb: repoops, permssweep, permsbackfill, mirrordone consumers (DLQ capture)
entire-api: analysis, ingest, jobsdlq (DLQ capture and the DLQ sweeper)

backoff's legacy bridges (NakOrTermLegacy, NumDeliveredLegacy, IsFinalDeliveryLegacy) — zero callers anywhere. Phase 2 built them anticipating mirror-pipeline's legacy consumers would adopt the policy, but Phase 3 kept those fetch loops (fanoutengine, worker) fully local, and every consumer that did adopt backoff is on the modern jetstream.Msg API. They're dead weight today and could be deleted from go-nuts without touching any service.

So the realistic cleanup order is:

backoff *Legacy set — deletable now, no migration needed.
LegacyPublisher — goes away when the six publisher call sites swap their wiring from nc.JetStream() to jetstream.New(nc) and the modern Publisher. Mechanical but touches connection wiring in both repos.
LegacyJetStream/DLQPublisher — last, because natsmsg.DeadLetter's own signature rides it; removing it means changing the shared DLQ API and all seven DLQ call sites across entiredb and entire-api.

One caveat on 2: the modern Publisher.JS field is the full jetstream.JetStream interface, so the swap also changes what tests fake — same friction that kept entire-api's forwarder publisher local. A narrow modern publish interface in natsmsg (mirroring LegacyJetStream) would make that migration clean; worth folding into the bridge-deletion ticket if you file it.