Reset Consumer correct API response and improve test

[scottf]

No description provided.

[scottf]

@claude please review

[synadia-claude-reviewer]

Claude finished @scottf's task in 2m 0s —— View job

---

Code Review

• Gather context (git diff, changed files)
• Analyze changes
• Post review feedback

---

This is a clean, well-motivated change. Here's the breakdown:

API Change — boolean → ConsumerInfo return type

JetStreamManagement.java / NatsJetStreamManagement.java

The core fix in NatsJetStreamManagement.java:399:

// Before (wrong)
return new SuccessApiResponse(resp).throwOnHasError().getSuccess();

// After (correct)
return new ConsumerInfo(resp).throwOnHasError();

This correctly reflects what the NATS server actually returns from the consumer reset endpoint — a full ConsumerInfo object, not a simple success/failure boolean. The ConsumerInfo extends ApiResponse<ConsumerInfo> pattern with throwOnHasError() returning T is already used consistently throughout the file (lines 43, 107, 175, 187). The return type change is a breaking API change, but it's necessary to match the server's actual response.

The updated Javadoc @return the current consumer after the reset is accurate and the method signatures are consistent.

Test Changes — JetStreamManagementTests.java

m.ack() → m.ackSync(Duration.ofSeconds(1))

Good improvement. Fire-and-forget ack() can cause race conditions in integration tests because the ack may not have been processed by the server when the test subsequently checks getNumAckPending(). ackSync waits for server confirmation, making assertions reliable.

sleep(200) additions

Pragmatic CI stability fix. The comment explains the intent clearly. A retry/polling loop would be more robust (the 200ms might be too short on a heavily loaded runner), but this is a reasonable tradeoff for test simplicity.

Removed ctx.getConsumerInfo() after second reset

Previously the test called resetConsumer() and then made a separate ctx.getConsumerInfo() call to check state. Now the return value of resetConsumer() is used directly:

// Before
jsm.resetConsumer(stream, consumer, 1);
ci = ctx.getConsumerInfo();

// After
ci = jsm.resetConsumer(stream, consumer, 1);

This is cleaner, eliminates an unnecessary network call, and directly validates the API contract of the new return type.

Assertions on reset return value

The test now immediately asserts numPending and numAckPending on the returned ConsumerInfo from the first reset, which previously wasn't validated at all. This is a meaningful test improvement.

Minor Notes

• The sleep(200) before the second getConsumerInfo() (line 1788) is on its own line, consistent with the others — no issue.
• No concerns about the sequence < 1 check in the payload construction — this correctly handles the no-sequence case the same as before.

Overall: Approve. The changes are correct, consistent with existing patterns, and the test improvements make the suite more reliable.

[Jarema]

LGTM!