correctly track begin seqnum when resend range ends with admin messages

[Copilot]

newBegin was computed after the resend loop by checking appMessageJustSent, but that flag was reset to false at the top of every iteration — so it only reflected whether the last message in the range was an app message. If the range ended with one or more admin messages after a resent app message, newBegin stayed at beginSeqNo instead of advancing past the last resent app message, producing a wrong NextExpectedMsgSeqNum or a spurious sequence reset.

A second related issue existed in the post-loop code: when a trailing admin gap-fill (generateSequenceReset) was emitted for a non-zero begin, newBegin was not advanced past it, causing the subsequent generateSequenceResetIfNeeded call to overlap the same sequence range. This affected both the enableNextExpectedMsgSeqNum=false branch and the enableNextExpectedMsgSeqNum=true branch — in the latter, an else guard caused the trailing gap-fill SequenceReset to be omitted entirely rather than emitted with a wrong sequence number.

Changes

• Session.java — resendMessages / ResendRequest handling loop

  • Move int newBegin = beginSeqNo declaration to before the loop
  • Update newBegin = msgSeqNum + 1 in-place at the point where an app message is resent (alongside the existing begin = 0)
  • Remove the now-redundant post-loop appMessageJustSent flag and its conditional block
  • Unify the enableNextExpectedMsgSeqNum and else post-loop branches: remove the else guard in the enableNextExpectedMsgSeqNum branch so generateSequenceResetIfNeeded is always called, and advance newBegin = msgSeqNum + 1 after any trailing admin gap-fill in both branches

• SessionTest.java

  • Add regression test resendMessages_whenLastRetrievedMessageIsAdmin_finalSequenceResetShouldNotOverlapResentAppMessages covering the scenario where the last retrieved persisted message is an admin message (Heartbeat) with enableNextExpectedMsgSeqNum=false
  • Add regression test resendMessages_whenLastRetrievedMessageIsAdmin_finalSequenceResetShouldNotOverlapResentAppMessages_withEnableNextExpectedMsgSeqNum covering the same scenario with enableNextExpectedMsgSeqNum=true
  • Both tests share a common doResendMessages_whenLastRetrievedMessageIsAdmin_scenario(boolean) driver

// Before: only correct if the last message in the range was an app message
int newBegin = beginSeqNo;
if (appMessageJustSent) {       // false if loop ended on admin msg
    newBegin = msgSeqNum + 1;
}

// After: updated at the exact point each app message is resent,
// and again after any trailing admin gap-fill; branches unified
begin = 0;
newBegin = msgSeqNum + 1;       // always reflects last resent app msg

// post-loop (same for both enableNextExpectedMsgSeqNum values):
if (begin != 0) {
    generateSequenceReset(receivedMessage, begin, msgSeqNum + 1);
    newBegin = msgSeqNum + 1;   // advance past the trailing admin gap-fill
}
generateSequenceResetIfNeeded(receivedMessage, newBegin, endSeqNo, msgSeqNum);

[chrjohn]

@copilot please copy the unit test from #1194 to this branch.

[Copilot]

@copilot please copy the unit test from #1194 to this branch.

Done — the regression test from PR #1194 has been added (commit test: add regression test for issue #344 and fix newBegin after trailing admin gap-fill).

Running it also exposed that the original fix was incomplete: after the post-loop generateSequenceReset(begin, msgSeqNum+1) that covers a trailing admin gap, newBegin wasn't advanced, causing generateSequenceResetIfNeeded to overlap the same range. Added newBegin = msgSeqNum + 1 after that call to fix it. All 74 SessionTest tests pass.

[chrjohn]

@copilot
please copy the unit test from #1194 to this branch. The test has been updated.

[Copilot]

@copilot
please copy the unit test from #1194 to this branch. The test has been updated.

Done. The updated test from PR #1194 has been copied (commit fix+test: unify post-loop branch and add enableNextExpectedMsgSeqNum regression test).

This also required a companion fix in Session.java: the enableNextExpectedMsgSeqNum=true post-loop branch had an else guard that silently skipped generateSequenceResetIfNeeded whenever begin != 0, so the trailing gap-fill SequenceReset was never emitted in that case. Removing the else and adding newBegin = msgSeqNum + 1 (same as the else-branch) makes both paths identical and the new withEnableNextExpectedMsgSeqNum test now passes alongside the existing one. All 75 SessionTest tests pass.