kvserver: fix false positive in 'outstanding reproposal' assertion

In cockroachdb#94633, I introduced[^1] an assertion that attempted to catch cases
in which we might otherwise accidentally end up applying a proposal
twice.

This assertion had a false positive, see the updated comment within.

I was able to reproduce the failure within ~minutes via
`./experiment.sh` in cockroachdb#97173 as of 33dcdef.

Closes cockroachdb#94633.

[^1]: https://github.com/cockroachdb/cockroach/pull/94633/files#diff-50e458584d176deae52b20a7c04461b3e4110795c8c9a307cf7ee6696ba6bc60R238

Epic: none
Release note: None

pkg/kv/kvserver/replica_application_state_machine.go

@@ -294,19 +294,46 @@ func (sm *replicaStateMachine) ApplySideEffects(
 			sm.r.handleReadWriteLocalEvalResult(ctx, *cmd.localResult)
 		}
 
-		if higherReproposalsExist := cmd.proposal.Supersedes(cmd.Cmd.MaxLeaseIndex); higherReproposalsExist {
-			// If the command wasn't rejected, we just applied it and no higher
-			// reproposal must exist (since that one may also apply).
+		if higherReproposalsExist := cmd.proposal.Supersedes(cmd.Cmd.MaxLeaseIndex); higherReproposalsExist &&
+			(!cmd.Rejected() || cmd.Rejection == kvserverbase.ProposalRejectionIllegalLeaseIndex) {
+			// If this entry was accepted, there must not be superseding reproposal,
+			// as nothing prevents it from applying as well in the common case.
 			//
-			// If the command was rejected with ProposalRejectionPermanent, no higher
-			// reproposal should exist (after all, whoever made that reproposal should
-			// also have seen a permanent rejection).
+			// If this entry was rejected but only on account of the lease applied
+			// index, then the call to tryReproposeWithNewLeaseIndex[^1] must have
+			// returned an error (or the proposal would not be IsLocal() now; we
+			// unbind on success). But that call, by design, does not an error for a
+			// proposal that is already superseded.
 			//
-			// If it was rejected with ProposalRejectionIllegalLeaseIndex, then the
-			// subsequent call to tryReproposeWithNewLeaseIndex[^1] must have returned an
-			// error (or the proposal would not be IsLocal() now). But that call
-			// cannot return an error for a proposal that is already superseded
-			// initially.
+			// If the command was rejected permanently, a superseding entry *can*
+			// actually exist, because the condition that leads to a permanent
+			// rejection now may not have been given at the time at which the
+			// superseding proposal was spawned.
+			//
+			// For example, consider the following initial situation:
+			//
+			//   [lease seq is 1]
+			//   idx 99: unrelated cmd at LAI 10000, lease seq = 1
+			//   idx 100: cmd X at LAI 10000, lease seq = 1
+			//   idx 100: cmd X at LAI 10000, lease seq = 1
+			//
+			// Command `X` at log position 100 will get reproposed with a new lease
+			// index (as idx 99 consumed the LAI). But now the lease might also change
+			// hands, and an identical reproposal of index 100 might be appended, too:
+			//
+			//   [lease seq is 1]
+			//   idx 99: unrelated cmd at LAI 10000, lease seq = 1
+			//   idx 100: cmd X at LAI 10000, lease seq = 1
+			//   idx 101: new lease seq=2
+			//   idx 102: cmd X at LAI 10000, lease seq = 1
+			//   idx 103: cmd X at LAI 20000, lease seq = 1
+			//
+			// When we apply index 102, we will see a permanent rejection but the
+			// proposal is local (since we don't unlink it if it is already superseded
+			// initially, see `retrieveLocalProposals`) and superseded (by idx 103). A
+			// permanent rejection dooms all reproposals (including the superseding
+			// one) to the same fate, so idx 103 will get rejected just like idx 102.
+			// is.
 			//
 			// [^1]: see (*replicaDecoder).retrieveLocalProposals()
 			sm.r.mu.RLock()