The version check logic is incorrect. When minVersion == maxVersion but both are
below V_3_7_0, the first condition is false so the || short-circuits and the check
passes incorrectly. The condition should use && (all nodes same version AND at or
above minimum) or restructure to check the minimum version independently of the
equality check.

server/src/main/java/org/opensearch/cluster/metadata/MetadataInPlaceSplitShardService.java [108-109]

-if (currentState.nodes().getMinNodeVersion().equals(currentState.nodes().getMaxNodeVersion()) == false
-    || currentState.nodes().getMinNodeVersion().before(Version.V_3_7_0)) {
+if (currentState.nodes().getMinNodeVersion().before(Version.V_3_7_0)
+    || currentState.nodes().getMinNodeVersion().equals(currentState.nodes().getMaxNodeVersion()) == false) {

The assertion using || with != conditions is a tautology — it is always true because
a state cannot simultaneously equal both INITIALIZING and RELOCATING. The original
code used && for the negated conditions, which correctly asserts that
expectedShardSize >= 0 when the state is any of those states. This should use && to
preserve the intended invariant.

server/src/main/java/org/opensearch/cluster/routing/ShardRouting.java [149-152]

 assert expectedShardSize >= 0
-    || state != ShardRoutingState.INITIALIZING
-    || state != ShardRoutingState.RELOCATING
-    || state != ShardRoutingState.SPLITTING : expectedShardSize + " state: " + state;
+    || (state != ShardRoutingState.INITIALIZING
+        && state != ShardRoutingState.RELOCATING
+        && state != ShardRoutingState.SPLITTING) : expectedShardSize + " state: " + state;

The writeTo method only writes splitChildAllocationIds and parentAllocationId when
the version is at or after V_3_7_0, but readOptionalStringList may return an empty
list (not null) when the collection was written as empty. The
Collections.unmodifiableList wrapping of an empty list is fine, but the null-check
should also handle the case where splitChildAllocationIds was written as an empty
list to avoid inconsistency with the null sentinel used elsewhere. Consider
normalizing empty lists to null on read.

server/src/main/java/org/opensearch/cluster/routing/AllocationId.java [122-129]

 if (in.getVersion().onOrAfter(Version.V_3_7_0)) {
     List<String> childIds = in.readOptionalStringList();
-    splitChildAllocationIds = childIds == null ? null : Collections.unmodifiableList(childIds);
+    splitChildAllocationIds = (childIds == null || childIds.isEmpty()) ? null : Collections.unmodifiableList(childIds);
     parentAllocationId = in.readOptionalString();
 } else {
     splitChildAllocationIds = null;
     parentAllocationId = null;
 }

The IN_PLACE_SPLIT_SHARD case is inserted before REMOTE_STORE in the switch
statement, but in the Type enum IN_PLACE_SPLIT_SHARD is declared after REMOTE_STORE.
The readFrom method uses Type.values()[in.readByte()] which relies on ordinal order,
so the switch case ordering doesn't matter for correctness here, but the enum
ordinal of IN_PLACE_SPLIT_SHARD (6) must match what was written. However, inserting
IN_PLACE_SPLIT_SHARD between LOCAL_SHARDS and REMOTE_STORE in the switch while it
has ordinal 6 (after REMOTE_STORE at ordinal 5) is fine for the switch, but the
RecoverySourceTests confirms ordinal 6. Verify that any persisted cluster state
using the old REMOTE_STORE byte value (5) is not broken by the new enum ordering
where IN_PLACE_SPLIT_SHARD is appended at ordinal 6.

server/src/main/java/org/opensearch/cluster/routing/RecoverySource.java [94-97]

+case REMOTE_STORE:
+    return new RemoteStoreRecoverySource(in);
 case IN_PLACE_SPLIT_SHARD:
     return InPlaceSplitShardRecoverySource.INSTANCE;
-case REMOTE_STORE:

The assert condition a || b || c || d where b, c, d are state != X will always be
true because a state cannot simultaneously be INITIALIZING, RELOCATING, and
SPLITTING — at least two of those != conditions will always be true. This makes the
assertion meaningless. The original code used && for the negated conditions, which
correctly asserts that expectedShardSize >= 0 when the state IS one of those values.

server/src/main/java/org/opensearch/cluster/routing/ShardRouting.java [149-152]

 assert expectedShardSize >= 0
-    || state != ShardRoutingState.INITIALIZING
-    || state != ShardRoutingState.RELOCATING
-    || state != ShardRoutingState.SPLITTING : expectedShardSize + " state: " + state;
+    || (state != ShardRoutingState.INITIALIZING
+        && state != ShardRoutingState.RELOCATING
+        && state != ShardRoutingState.SPLITTING) : expectedShardSize + " state: " + state;

The writeTo method writes splitChildAllocationIds using
writeOptionalStringCollection, but the readFrom reads it back using
readOptionalStringList. These two methods may not be symmetric —
writeOptionalStringCollection writes a Collection while readOptionalStringList
returns a List. Verify that these are compatible; if writeOptionalStringCollection
writes a null marker differently than readOptionalStringList expects,
deserialization will be corrupted. It is safer to use a consistent pair such as
writeOptionalStringArray/readOptionalStringArray or ensure the stream format
matches.

server/src/main/java/org/opensearch/cluster/routing/AllocationId.java [119-130]

 AllocationId(StreamInput in) throws IOException {
     this.id = in.readString();
     this.relocationId = in.readOptionalString();
     if (in.getVersion().onOrAfter(Version.V_3_7_0)) {
         List<String> childIds = in.readOptionalStringList();
         splitChildAllocationIds = childIds == null ? null : Collections.unmodifiableList(childIds);
         parentAllocationId = in.readOptionalString();
     } else {
         splitChildAllocationIds = null;
         parentAllocationId = null;
     }
 }
 
+@Override
+public void writeTo(StreamOutput out) throws IOException {
+    out.writeString(this.id);
+    out.writeOptionalString(this.relocationId);
+    if (out.getVersion().onOrAfter(Version.V_3_7_0)) {
+        out.writeOptionalStringList(splitChildAllocationIds);  // use writeOptionalStringList for symmetry
+        out.writeOptionalString(parentAllocationId);
+    }
+}
+

The version check logic is incorrect. When minVersion != maxVersion (mixed cluster),
the second condition minVersion.before(V_3_7_0) is not evaluated due to
short-circuit OR, so a mixed cluster where both nodes are >= V_3_7_0 would still be
rejected. The intent appears to be: reject if mixed-version OR if min version is too
old. The current logic actually does this correctly via OR, but the error message
says "same version" which is misleading when the real failure is the version being
too old. More critically, a mixed cluster where min >= V_3_7_0 should arguably also
be rejected, but the current logic rejects it — which may be intentional. However,
the two conditions should be separated with distinct error messages to avoid
confusion.

server/src/main/java/org/opensearch/cluster/metadata/MetadataInPlaceSplitShardService.java [108-113]

-if (currentState.nodes().getMinNodeVersion().equals(currentState.nodes().getMaxNodeVersion()) == false
-    || currentState.nodes().getMinNodeVersion().before(Version.V_3_7_0)) {
+if (currentState.nodes().getMinNodeVersion().equals(currentState.nodes().getMaxNodeVersion()) == false) {
+    throw new IllegalArgumentException(
+        "In-place shard split requires all nodes to be on the same version"
+    );
+}
+if (currentState.nodes().getMinNodeVersion().before(Version.V_3_7_0)) {
+    throw new IllegalArgumentException(
+        "In-place shard split requires all nodes to be on the same version, at or above " + Version.V_3_7_0
+    );
+}

The IN_PLACE_SPLIT_SHARD type is appended at the end of the enum, but readFrom uses
Type.values()[in.readByte()] which relies on ordinal order. Adding
IN_PLACE_SPLIT_SHARD after REMOTE_STORE changes the ordinal of nothing existing,
which is fine. However, REMOTE_STORE was previously the last entry and its ordinal
(5) is preserved. The readFrom switch uses named cases, not ordinals, so this is
safe. But the writeTo in RecoverySource writes getType().ordinal() as a byte —
verify that IN_PLACE_SPLIT_SHARD (ordinal 6) is correctly handled in readFrom since
the switch case was added. This appears correct, but the @ExperimentalApi annotation
is missing on InPlaceSplitShardRecoverySource while the class javadoc says
@opensearch.experimental.

server/src/main/java/org/opensearch/cluster/routing/RecoverySource.java [259-263]

-public enum Type {
-    EMPTY_STORE,
-    EXISTING_STORE,
-    PEER,
-    SNAPSHOT,
-    LOCAL_SHARDS,
-    REMOTE_STORE,
-    IN_PLACE_SPLIT_SHARD
-}
+/**
+ * Recovery of child shards from a source shard on the same node during in-place shard split.
+ *
+ * @opensearch.experimental
+ */
+@ExperimentalApi
+public static class InPlaceSplitShardRecoverySource extends RecoverySource {

The readOptionalStringList method may not exist on StreamInput — the standard
OpenSearch streaming API typically uses readOptionalWriteable or reads collections
differently. If readOptionalStringList is not a valid method, this will cause a
compilation or runtime error. The corresponding writeTo uses
writeOptionalStringCollection, so the read side should use a compatible method.
Verify that readOptionalStringList is a valid StreamInput method; if not, use
in.readBoolean() ? in.readStringList() : null or the appropriate paired method.

server/src/main/java/org/opensearch/cluster/routing/AllocationId.java [119-130]

-AllocationId(StreamInput in) throws IOException {
-    this.id = in.readString();
-    this.relocationId = in.readOptionalString();
-    if (in.getVersion().onOrAfter(Version.V_3_7_0)) {
-        List<String> childIds = in.readOptionalStringList();
-        splitChildAllocationIds = childIds == null ? null : Collections.unmodifiableList(childIds);
-        parentAllocationId = in.readOptionalString();
-    } else {
-        splitChildAllocationIds = null;
-        parentAllocationId = null;
-    }
+if (in.getVersion().onOrAfter(Version.V_3_7_0)) {
+    boolean hasChildIds = in.readBoolean();
+    List<String> childIds = hasChildIds ? in.readStringList() : null;
+    splitChildAllocationIds = childIds == null ? null : Collections.unmodifiableList(childIds);
+    parentAllocationId = in.readOptionalString();
+} else {
+    splitChildAllocationIds = null;
+    parentAllocationId = null;
 }

The routingTableBuilder is created but no modifications are made to it before
calling build(). The routing table is built unchanged, and the reroute is expected
to handle allocation of child shards. However, if the intent is to add child shard
routing entries to the routing table (e.g., marking the parent shard as SPLITTING
and adding child shards as INITIALIZING), this needs to happen before building the
routing table. If the reroute function handles this entirely, it should be
documented; otherwise, the routing table update for the split state transition is
missing.

server/src/main/java/org/opensearch/cluster/metadata/MetadataInPlaceSplitShardService.java [145-154]

-RoutingTable.Builder routingTableBuilder = RoutingTable.builder(currentState.routingTable());
 Metadata.Builder metadataBuilder = Metadata.builder(currentState.metadata());
 IndexMetadata.Builder indexMetadataBuilder = IndexMetadata.builder(curIndexMetadata);
 
 SplitShardsMetadata.Builder splitMetadataBuilder = new SplitShardsMetadata.Builder(splitShardsMetadata);
 splitMetadataBuilder.splitShard(shardId, request.getSplitInto());
 indexMetadataBuilder.splitShardsMetadata(splitMetadataBuilder.build());
 
-RoutingTable routingTable = routingTableBuilder.build();
+// TODO: Update routing table to reflect SPLITTING state for parent shard and add child shards
+RoutingTable routingTable = RoutingTable.builder(currentState.routingTable()).build();
 metadataBuilder.put(indexMetadataBuilder);

The version check logic is incorrect. When minVersion.equals(maxVersion) is false
(mixed cluster), the condition short-circuits and throws, which is correct. However,
when all nodes are on the same version but that version is before V_3_7_0, the
before check correctly throws. But if minVersion.equals(maxVersion) is true and the
version is at or above V_3_7_0, the condition is false || false = false, which is
correct. The real issue is that a mixed-version cluster where all nodes happen to be
below V_3_7_0 would only hit the first condition — but actually the logic is subtly
wrong: a uniform cluster below V_3_7_0 satisfies equals == true, so the first part
is false, and then before(V_3_7_0) is true, so it throws correctly. However, a mixed
cluster where min is above V_3_7_0 but versions differ would throw due to the first
condition, which may be overly strict. More critically, the condition should use ||
between the two checks but the intent should be: throw if mixed OR if below minimum
version. The current logic is actually correct but the equals check for version
uniformity should use compareTo or check both min and max against V_3_7_0 to be
clearer and avoid subtle bugs.

server/src/main/java/org/opensearch/cluster/metadata/MetadataInPlaceSplitShardService.java [108-109]

-if (currentState.nodes().getMinNodeVersion().equals(currentState.nodes().getMaxNodeVersion()) == false
-    || currentState.nodes().getMinNodeVersion().before(Version.V_3_7_0)) {
+if (currentState.nodes().getMinNodeVersion().before(Version.V_3_7_0)
+    || currentState.nodes().getMinNodeVersion().equals(currentState.nodes().getMaxNodeVersion()) == false) {

The Type enum uses ordinal values (via Type.values()[in.readByte()]) for
serialization in readFrom. Adding IN_PLACE_SPLIT_SHARD at the end is safe for new
clusters, but REMOTE_STORE was previously the last entry at ordinal 5. The new
IN_PLACE_SPLIT_SHARD is at ordinal 6. However, the readFrom method uses
Type.values()[in.readByte()] which relies on ordinal position — if any older
serialized data or mixed-version nodes send byte value 5 expecting REMOTE_STORE,
this still works. But the test testTypeEnumOrdinalStability in
RecoverySourceSplitTests only checks that IN_PLACE_SPLIT_SHARD is last, without
verifying REMOTE_STORE remains at ordinal 5. The existing
RecoverySourceTests.testRecoverySourceTypeOrder does verify this. This is fine, but
the readFrom switch should be verified to handle the new type correctly — currently
it does via the added case IN_PLACE_SPLIT_SHARD. No code change needed here, but the
writeTo method in InPlaceSplitShardRecoverySource is inherited and should write the
type byte — verify the base class writeTo writes getType().ordinal() as a byte
correctly for ordinal 6 which fits in a byte.

server/src/main/java/org/opensearch/cluster/routing/RecoverySource.java [122-130]

+public enum Type {
+    EMPTY_STORE,
+    EXISTING_STORE,
+    PEER,
+    SNAPSHOT,
+    LOCAL_SHARDS,
+    REMOTE_STORE,
+    IN_PLACE_SPLIT_SHARD
+}
 
-

The routingTableBuilder is created but no modifications are made to it before
calling build(). The routing table update (e.g., adding child shard entries or
updating the parent shard state to SPLITTING) appears to be missing. Without
updating the routing table, the reroute call may not correctly allocate child shards
since they won't have routing entries.

server/src/main/java/org/opensearch/cluster/metadata/MetadataInPlaceSplitShardService.java [145-154]

-RoutingTable.Builder routingTableBuilder = RoutingTable.builder(currentState.routingTable());
-Metadata.Builder metadataBuilder = Metadata.builder(currentState.metadata());
-IndexMetadata.Builder indexMetadataBuilder = IndexMetadata.builder(curIndexMetadata);
-
-SplitShardsMetadata.Builder splitMetadataBuilder = new SplitShardsMetadata.Builder(splitShardsMetadata);
-splitMetadataBuilder.splitShard(shardId, request.getSplitInto());
-indexMetadataBuilder.splitShardsMetadata(splitMetadataBuilder.build());
-
+// routingTableBuilder should be updated to reflect the new split state
+// e.g., update parent shard to SPLITTING state and add unassigned child shards
+// before building the routing table
 RoutingTable routingTable = routingTableBuilder.build();
 metadataBuilder.put(indexMetadataBuilder);

The version check uses equals to verify all nodes are on the same version, but this
is overly strict. Two nodes could be on different versions both at or above V_3_7_0
and the split should still be allowed. The condition should only reject if the
minimum node version is below V_3_7_0, regardless of whether all nodes are on the
same version.

server/src/main/java/org/opensearch/cluster/metadata/MetadataInPlaceSplitShardService.java [108-109]

-if (currentState.nodes().getMinNodeVersion().equals(currentState.nodes().getMaxNodeVersion()) == false
-    || currentState.nodes().getMinNodeVersion().before(Version.V_3_7_0)) {
+if (currentState.nodes().getMinNodeVersion().before(Version.V_3_7_0)) {

The equals method now includes splitChildAllocationIds and parentAllocationId, but
these fields are described as "transient" in ShardRouting (not serialized on the
wire for older versions). This means two AllocationId objects that are logically
equal (same id and relocationId) but differ in split fields (e.g., one deserialized
from an old node) will not be considered equal. This could break allocation
tracking. Consider whether split fields should be excluded from equals/hashCode, or
document this behavior explicitly.

server/src/main/java/org/opensearch/cluster/routing/AllocationId.java [271-283]

-AllocationId(StreamInput in) throws IOException {
-    this.id = in.readString();
-    this.relocationId = in.readOptionalString();
-    if (in.getVersion().onOrAfter(Version.V_3_7_0)) {
-        List<String> childIds = in.readOptionalStringList();
-        splitChildAllocationIds = childIds == null ? null : Collections.unmodifiableList(childIds);
-        parentAllocationId = in.readOptionalString();
-    } else {
-        splitChildAllocationIds = null;
-        parentAllocationId = null;
-    }
+@Override
+public boolean equals(Object o) {
+    if (this == o) return true;
+    if (!(o instanceof AllocationId)) return false;
+    AllocationId that = (AllocationId) o;
+    return Objects.equals(id, that.id)
+        && Objects.equals(relocationId, that.relocationId);
 }
 
+@Override
+public int hashCode() {
+    return Objects.hash(id, relocationId);
+}
+

The Type enum uses ordinal values (via Type.values()[in.readByte()]) for
serialization in readFrom. Adding IN_PLACE_SPLIT_SHARD at the end is safe, but
REMOTE_STORE was previously the last entry at ordinal 5. Any existing serialized
data with REMOTE_STORE (ordinal 5) will still deserialize correctly, but the switch
statement in readFrom handles IN_PLACE_SPLIT_SHARD before REMOTE_STORE, which is
fine. However, the test testTypeEnumOrdinalStability in RecoverySourceSplitTests
asserts IN_PLACE_SPLIT_SHARD is last — this is correct and should be maintained. No
code change needed here, but ensure the readFrom switch handles the new type before
the default case, which it does.

server/src/main/java/org/opensearch/cluster/routing/RecoverySource.java [122-130]

+public enum Type {
+    EMPTY_STORE,
+    EXISTING_STORE,
+    PEER,
+    SNAPSHOT,
+    LOCAL_SHARDS,
+    REMOTE_STORE,
+    IN_PLACE_SPLIT_SHARD
+}
 
-

The routingTableBuilder is created but never modified before calling .build(), so
the routing table in the updated cluster state is identical to the current one.
Child shards need to be added to the routing table here (before the reroute) so the
allocation service can assign them. Without this, the reroute call has no new
unassigned shards to allocate.

server/src/main/java/org/opensearch/cluster/metadata/MetadataInPlaceShardSplitService.java [145-154]

 RoutingTable.Builder routingTableBuilder = RoutingTable.builder(currentState.routingTable());
 Metadata.Builder metadataBuilder = Metadata.builder(currentState.metadata());
 IndexMetadata.Builder indexMetadataBuilder = IndexMetadata.builder(curIndexMetadata);
 
 SplitShardsMetadata.Builder splitMetadataBuilder = new SplitShardsMetadata.Builder(splitShardsMetadata);
 splitMetadataBuilder.splitShard(shardId, request.getSplitInto());
-indexMetadataBuilder.splitShardsMetadata(splitMetadataBuilder.build());
+SplitShardsMetadata updatedSplitMetadata = splitMetadataBuilder.build();
+indexMetadataBuilder.splitShardsMetadata(updatedSplitMetadata);
+
+// Add child shards as unassigned entries in the routing table so the allocator can assign them
+// (child shard routing entries should be added here based on updatedSplitMetadata)
 
 RoutingTable routingTable = routingTableBuilder.build();
 metadataBuilder.put(indexMetadataBuilder);

The writeTo method writes splitChildAllocationIds using
writeOptionalStringCollection, but the readFrom uses readOptionalStringList. These
two methods may not be symmetric — writeOptionalStringCollection writes a Collection
which may not guarantee list ordering or the same wire format as
readOptionalStringList expects. Use matching read/write pairs (e.g.,
writeOptionalStringList / readOptionalStringList) to ensure correct deserialization.

server/src/main/java/org/opensearch/cluster/routing/AllocationId.java [122-129]

+if (in.getVersion().onOrAfter(Version.V_3_6_0)) {
+    List<String> childIds = in.readOptionalStringList();
+    splitChildAllocationIds = childIds == null ? null : Collections.unmodifiableList(childIds);
+    parentAllocationId = in.readOptionalString();
+} else {
+    splitChildAllocationIds = null;
+    parentAllocationId = null;
+}
 
-

IN_PLACE_SHARD_SPLIT is added after REMOTE_STORE in the enum but is handled before
REMOTE_STORE in the switch statement. While functionally correct, the readFrom
switch uses Type.values()[in.readByte()] (ordinal-based), so the enum ordinal order
is what matters for wire compatibility. The enum has IN_PLACE_SHARD_SPLIT at ordinal
6 (after REMOTE_STORE at 5), but the RecoverySourceTests confirms this. Ensure the
switch case order matches the enum declaration order to avoid future confusion and
maintenance errors.

server/src/main/java/org/opensearch/cluster/routing/RecoverySource.java [94-97]

+case REMOTE_STORE:
+    return new RemoteStoreRecoverySource(in);
 case IN_PLACE_SHARD_SPLIT:
     return InPlaceShardSplitRecoverySource.INSTANCE;
-case REMOTE_STORE:

The version check uses equals to verify all nodes are on the same version, but this
is overly strict — it rejects valid homogeneous clusters where min equals max but
both are above V_3_6_0. The real intent is to reject mixed-version clusters AND
clusters below the minimum version. The condition should be: reject if minVersion !=
maxVersion OR if minVersion < V_3_6_0, which is what the code does — however, the
equals check is correct for same-version enforcement. The real bug is that a cluster
where all nodes are on V_3_6_0 but minVersion.equals(maxVersion) is true would still
pass, so this is fine. But using equals on Version objects should be verified to
work correctly vs. using onOrAfter/before comparisons for consistency and
correctness.

server/src/main/java/org/opensearch/cluster/metadata/MetadataInPlaceShardSplitService.java [108-109]

-if (currentState.nodes().getMinNodeVersion().equals(currentState.nodes().getMaxNodeVersion()) == false
-    || currentState.nodes().getMinNodeVersion().before(Version.V_3_6_0)) {
+if (currentState.nodes().getMinNodeVersion().before(Version.V_3_6_0)
+    || currentState.nodes().getMinNodeVersion().equals(currentState.nodes().getMaxNodeVersion()) == false) {