JetStream Broker Semantics Test Suite Verification

Implemented COR-1257 on branch nodo/cor-1257-...: internal/brokersemantics, 21 tests against an embedded nats-server at the go.mod pin (v2.14.3). Build, vet, golangci-lint (0 issues), and go test -race ./... all green; the suite adds ~4s and carries no build tag, so mise run test:ci already gates it.

Three of the beliefs I was asked to assert turned out to be false — these change decisions:

1. Term settles. ENT-1492's and COR-944's premise ("a Term does NOT delete it, verified live on 2.14.2") does not reproduce — on either version. I ran the same test against an embedded 2.14.2 from the module cache: Term settles immediately, the floor advances, NumAckPending→0, on limits/workqueue/interest alike; on workqueue and interest it also removes the message. So backoff.TermOnExhaustion's doc is correct as written, and the fleet manifests' note is wrong. Dead-lettering is still right — because it keeps the payload, not because Term leaks.

2. A mechanism that does produce ENT-1492's symptom: Ack/Nak/Term are fire-and-forget publishes to $JS.ACK.>. With an identity lacking that publish grant (the COR-1224 gap on five consumers), msg.Ack() returns nil while the server rejects it — floor pinned, nothing logged, only an async error on the connection handler. DoubleAck reports it, as context.DeadlineExceeded, not nats.ErrTimeout. This makes backoff's "a disposition error is worth a log line but nothing more" an understatement: there is no error.

3. The NAK-delay rule is sharper than "BackOff governs timeouts, not Naks". Measured to the millisecond: effective delay = requested d + (BackOff[rung] − BackOff[0]). A flat ladder hides this; on [200ms, 1200ms], NakWithDelay(50ms) redelivered at 50ms, then 1050ms, then 1050ms. So a client-side delay cannot express its own envelope on a growing ladder — and this is the arithmetic behind ENT-1535's unverified "~78s apart against a 5m first rung".

Also pinned, as measured: the tail-repeat rule (wait before delivery N = BackOff[min(N-2, len-1)], so MaxDeliver == len(BackOff) leaves the last rung dead config); NumDelivered climbing to MaxDeliver with no disposition at all; an exhausted message pinning the floor with NumAckPending 0; the floor jumping to the delivered high-water mark on stream-side removal (ENT-1535's break-glass), after which floor+1 names nothing real; on a filtered consumer floor+1 naming an innocent unmatched sequence until an unrelated ack below the blocker flips it; AckWait silently rewritten to BackOff[0] on the ordinary path but rejected under pedantic (10157, driven via a raw $JS.API request since nats.go has no pedantic option); len(BackOff) > MaxDeliver rejected (10116) while == is accepted; a stream's ConsumerLimits silently filling jsconsumer's zero MaxAckPending/InactiveThreshold.

Two supporting changes: natsmsgtest's package doc now records the demotion to library-logic-only with the reason, and README gains a "fake vs real broker" section including how to read failures after a server bump. .golangci.yaml's ireturn allow-list gained jetstream.Stream/Consumer alongside the existing jetstream.JetStream entry.

I have not committed. Two things outside this repo follow from the findings: ENT-1492/COR-944 and the fleet manifest comments need correcting, and if the A1 breaker treats a nil Ack() as settlement, that's a hole worth closing — I've sent both to the go-nuts-ef session.

Handled all three; suite still green (build, vet, lint 0 issues, -race).

1. DoubleAck error identity — already correct, no change. The assertion was written as errors.Is(err, context.DeadlineExceeded) with a failure message calling out that a nats-sentinel-only classifier misses it. I extended that test's doc with the part their OutcomeUncertain reasoning makes explicit: a lost confirmation is indistinguishable from an ack that landed, so it means unknown, not not settled, and a caller must not re-dispatch on it. Stated as the broker-level rule rather than naming their unmerged symbols.

2. Flat-ladder blindness — generalized into the package doc as a "Fixtures that cannot fail" section, with their 3s/3s/3s as the worked example: the stretch term is exactly zero on a flat ladder, so such a fixture passes whether the stretch exists or not — against a live server — while pinning nothing. The rule for anyone adding cases: pick values that differ under the belief being rejected, and prefer asserting a measured quantity over asserting a call returned no error, since several of these semantics are calls that return no error while doing nothing.

3. ConsumerLimits read-back check — not added, and it needs your call. I agree with them that it's the natural fix and that it's new behaviour rather than a test. My suite pins the inheritance (including through jsconsumer.Start); nothing enforces that what you asked for is what the server stored.

One thing worth knowing before you scope it: InactiveThreshold/MaxAckPending inheritance is one of three silent rewrites on that same path — AckWait is overwritten with BackOff[0], and MaxDeliver 0 becomes −1. A single requested-vs-stored comparison after CreateOrUpdateConsumer would catch all three, which argues for one check rather than a per-field guard. Two questions for you:

• Do you want that comparison in jsconsumer (log-and-continue, or a hard error on mismatch), or is it admission-control's job on the CR side under COR-1255?
• Should I post the finding as a comment on COR-1257, open its own ticket, or leave Linear alone? I haven't written anything to Linear — that's a shared surface, so it's your call, not mine.

Also unresolved from earlier and still yours to decide: nothing is committed. My branch is nodo/cor-1257-... off main — the new package plus three small edits (natsmsgtest doc, README section, one ireturn allow-list entry) — and it lands cleanly beside their f4e160a in either order. Their branch has a non-compiling commit mid-history (6143624, fixed in e150438), so a bisect across it won't work until the squash they're waiting on you for.

No action needed from that — they've confirmed their side and I'm making no further changes. One refinement worth correcting in what I told you, since it changes how the follow-up should be scoped.

I said a single requested-vs-stored comparison would catch all three silent rewrites, which implied all three can reach a jsconsumer durable. Only one can, today:

• MaxDeliver 0 → −1: unreachable. Start sends EffectiveMaxDeliver(), so the server never sees the zero. My TestJSConsumerDefaultsLandOnTheServer already pins that guard — if it ever regressed, the suite goes red and the rewrite becomes reachable.
• AckWait ≠ BackOff[0]: unreachable, but by prevention rather than construction. On main, jsconsumer never sets BackOff at all; on their branch they report a validation that rejects the mismatch at Start. Their point — which I'd defer to them on, since I can't see that symbol from my tree — is that this is a check someone could later judge too strict, and the rewrite returns if it softens.
• ConsumerLimits inheritance: the only one that gets through today, which is why neither of us had it.

That shifts the follow-up's justification: the read-back check is a backstop against future softening and against whatever NATS starts normalizing next, not a fix for three live bugs. Which argues for comparing the whole requested-vs-stored config rather than an enumerated field list — an enumerated list only catches the rewrites we already know about.

Still waiting on you, unchanged: whether that check belongs in jsconsumer or in COR-1255 admission control; whether to record the finding on COR-1257 or open its own ticket (I haven't written to Linear); and whether to commit the branch.

All four addressed. mise run test:ci, mise run lint (0 issues), mise run build, go mod tidy -diff, gofmt, git diff --check, and go test -race -count=5 ./internal/brokersemantics all pass.

[P2] NakOrTerm / Outcome — Outcome is now documented as the disposition attempted, not one the broker confirmed. NakOrTerm states that Nak and Term are fire-and-forget publishes to $JS.ACK.>, that nil means only "written to the connection", and that without the $JS.ACK.> grant every disposition returns nil while the floor silently stops advancing — surfacing only on the connection's async error handler, so a consumer needs both the grant and nuts.Connect's async-error logging. It also states the DoubleAck rule: its error means unknown, not not settled, so it isn't grounds to re-dispatch. A returned error is now framed as the lesser failure.

[P2] Config zero values — InactiveThreshold and MaxAckPending now say zero holds only absent stream ConsumerLimits, with a shared note on the consequence the field docs were hiding: an inherited deletion timer, and a durable deleted that way replays from DeliverPolicy. The validation text now reads 0 (server or stream ConsumerLimits default).

[P2] Policy / DelayFor — both now carry the incompatibility, DelayFor with the measured formula requested + (BackOff[rung] − BackOff[0]), clamped to the last rung, and the consequence that a caller metering DelayFor under a growing ladder publishes an envelope the broker never served. The package doc's "pin or meter the envelope" claim is qualified in the same terms.

[P3] Version attribution — replaced with a TestMain that writes the version to stderr only when the run fails. Verified both directions non-verbose: a deliberate failure printed FAIL brokersemantics: measured against embedded nats-server 2.14.3 …; the green run is silent. Probe reverted.

Status, for the shipping decision. All assertion classes are written and green: the seven model-errors (Term settlement, blocker identity, NumDelivered inflation, BackOff-vs-Nak, absent-ladder, tail-repeat, non-positive MaxDeliver) plus the empirical corrections (normalization vs pedantic on both paths, the 10116 >/== boundary, BackOff with MaxDeliver=-1) and four findings neither list had: the silent $JS.ACK denial, NumAckPending 0 on exhaustion, ConsumerLimits inheritance, floor advance on stream-side removal. 21 tests, ~4s, no build tag. Nothing remains and nothing is blocking.

The split is clean where it matters, and not where it doesn't. The suite is entirely category (a): it imports only symbols already on main — backoff, jsconsumer.{Config,Start,Runner,Effective*,Default*}, natsmsg.{DeadLetter,KeepInProgress,SubjectToken} — and nothing from floor.go, retry.go, or Schedule. It compiles and passes on main alone, so it can merge immediately, independent of the A1 review.

The doc corrections are what overlap, in two files:

• jsconsumer/jsconsumer.go — f4e160a added its own ConsumerLimits note immediately above the same field. Same finding, two wordings. I folded their one distinct point ("read the durable's stored config") into my block, so resolution is now mechanical: keep mine, delete their 7-line note. Mine additionally carries the replay consequence, the pedantic angle, the test reference, and the validation-text fix the review asked for.
• README.md — their hunk spans the region mine sits in; a normal textual merge.

backoff/backoff.go, .golangci.yaml, natsmsgtest/fakemsg.go (they touch the struct, I touch the package doc), and the new package all merge cleanly.

Still nothing committed — my work is working-tree only on nodo/cor-1257-go-nuts-embedded-real-broker-test-suite-verify-jetstream off main; the suite tests are not on their branch, only their doc corrections are. If you want (a) on main now, say so and I'll commit; that ordering also makes their side's conflict resolution the mechanical one described above rather than mine.

Committed on nodo/cor-1257-go-nuts-embedded-real-broker-test-suite-verify-jetstream, two commits, working tree clean:

• 3b6f979 — the suite (internal/brokersemantics, 21 tests), the natsmsgtest demotion, the README split, and the one ireturn allow-list entry.
• f6f999f — the three public-doc corrections from your review (backoff.Outcome/NakOrTerm, Policy/DelayFor, jsconsumer.Config zero values plus the one validation string). Comments only.

Both are green individually — I checked 3b6f979 alone in a throwaway worktree (build, vet, lint 0 issues, tests), so bisect works across mine. Entire captured checkpoints automatically (56b4d6813b46, 851f01f4a88c); HEAD passes the full -race run.

I split it that way deliberately: the history then states the causal order the exercise is about — the suite measures, then the docs it falsified get corrected — and it isolates the entire file overlap with the A1 branch into the second commit.

Not pushed — you asked to commit. Say so and I'll push; that's also what the orchestrator needs before the merge review, and mine-landing-first is the cheaper order (it reduces the A1 branch's conflict to "keep mine, delete their 7-line ConsumerLimits note", which I pre-folded their one distinct point into).

Two decisions still open, neither gating the merge: whether the requested-vs-stored config read-back check belongs in jsconsumer or in COR-1255 admission control, and whether the ConsumerLimits finding gets a comment on COR-1257 or its own ticket. I haven't written anything to Linear.

Both fixed, committed as 2683ab7. You were right on both counts, and they're the same failure — the "fixtures that cannot fail" trap the suite's own doc.go warns about, inside the suite that warns about it.

KeepInProgress cap. Nothing calls stop() now. The handler starts the heartbeat and returns without disposing — the wedged shape — so the only thing that can release the delivery is the cap firing on its own. It returns rather than sleeps because nats.go dispatches consume callbacks serially, so a sleeping handler would block the very redelivery under test; the extension doesn't depend on the handler still running, since InProgress moves the server's timer rather than holding a client-side lease. It asserts the extension holds at 3×AckWait, then that the reclaim lands near 6×AckWait — 15 ticks at AckWait/3 is 5×AckWait, plus one AckWait for the last extension to expire. keepInProgressMaxTicks is unexported, so the test states the arithmetic instead of importing it.

Permission coverage. Now table-driven over Ack, Nak, NakWithDelay, Term and InProgress — each on its own durable and message, so one silent failure can't mask another — plus DoubleAck as the surface that does report the rejection.

That rewrite surfaced a third instance of the same trap: the subtests run in parallel on one connection, so counting async violations before/after would let a sibling's violation satisfy this one's assertion. Violations are now attributed by matching the durable named in the rejected $JS.ACK subject, which is parallel-safe and a stronger claim than a count.

Both were mutation-checked, since a fixture asserted to gate something should be shown to fail when that thing is removed:

• keepInProgressMaxTicks raised to effectively unlimited → heartbeat test red (saw [{stream:1 consumer:1 delivered:1}] deliveries, want at least 2 — no redelivery ever arrives).
• $JS.ACK.> added to the allow-list → every disposition subtest red (floor advances, nothing stays pending, no violation reported).

Mutations reverted and verified (keepInProgressMaxTicks = 15 restored; the two remaining $JS.ACK.> occurrences in the file are prose). Validation after: go build, go vet, golangci-lint 0 issues, gofmt clean, go test -race ./... green, and -count=3 on the suite for flakiness.

Three commits now on the branch, still unpushed. The mutation-check discipline is worth making explicit in doc.go's "Fixtures that cannot fail" section — right now it says how to pick values but not "prove it fails when the thing under test is removed". Want me to add that, or leave the section as is?

Fixed in d9e2e0a. You were right, and the comment I'd written asserting siblings couldn't cross-satisfy was itself the false claim.

The match is now "." + durable + "." against the rejected subject, which is $JS.ACK.<stream>.<durable>.<delivered>.<stream seq>.<consumer seq>.<ts>.<pending> — so the durable is always a dot-delimited token. I chose the token match over renaming the durables because renaming fixes today's collision only; the bounded match holds however the fixture's names change later.

Demonstrated both directions, with plain Nak's disposition replaced by a no-op so that only the attribution could fail (its nil-return, floor and pending assertions still hold):

• Bounded token → Nak subtest red, naming the token it couldn't find and listing denied_nakdelay's violation among what it saw.
• Substring match restored → whole test passes. Your false positive, reproduced.

I also took the doc.go step I'd asked you about last turn rather than asking twice: "Fixtures that cannot fail" now requires breaking the thing a case claims to gate and watching it go red, and records the three fixtures review has caught doing nothing here — the heartbeat that stopped its own heartbeat, the Ack-only permission test, and this attribution. Three rounds of the same defect is enough evidence that the discipline belongs in the charter rather than in my habits. If you'd rather that paragraph stayed out, it's one commit to drop.

Validation: go build, go vet, golangci-lint 0 issues, gofmt, go mod tidy -diff, git diff --check, go test -race ./..., and -count=3 on the suite.

Four commits on nodo/cor-1257-..., tree clean, still unpublished — which is why there's no PR or CI state for you to review against. Want me to push? That's the remaining step before the merge-order plan (mine first, leaving the A1 branch the mechanical ConsumerLimits resolution) can actually execute.