From 26e9155e646f49a0ecb4461081b905fe2bcc09a4 Mon Sep 17 00:00:00 2001
From: Tobias Fischer <info@tobiasfischer.info>
Date: Sat, 5 Sep 2026 07:45:20 +1000
Subject: [PATCH] fix: close if-block before starting a new one in
 sort_vinca_lists

Two "- if:" blocks placed back-to-back with no blank line or comment
between them were parsed as a single block, since the "then:" handler
never resets in_then between blocks. Sorting then pooled both blocks'
then-items into one flat list and redistributed them by original line
position, which can move an item across a platform-condition boundary
into the wrong block.

Reproduced downstream in RoboStack/ros-rolling: adding entries to a
"not wasm32 and not win" block right before an adjacent, comment-less
"linux and not aarch64" block caused webots_ros2 (linux-only) to swap
places with an item that should have stayed unconditional.

Close the current if-block whenever a new "- if:" line is seen, the
same way a list-level comment already does, so each block sorts within
its own scope regardless of adjacency.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---
 vinca/sort_vinca_lists.py      | 18 +++++--
 vinca/test_sort_vinca_lists.py | 98 ++++++++++++++++++++++++++++++++++
 2 files changed, 113 insertions(+), 3 deletions(-)
 create mode 100644 vinca/test_sort_vinca_lists.py

diff --git a/vinca/sort_vinca_lists.py b/vinca/sort_vinca_lists.py
index b472288..2db7d4e 100644
--- a/vinca/sort_vinca_lists.py
+++ b/vinca/sort_vinca_lists.py
@@ -97,9 +97,21 @@ def sort_vinca_lists(path: Path) -> bool:
 
                 # Start of conditional block: "  - if: ..."
                 if RE_IF_BLOCK.match(line):
-                    if current_if_block is None:
-                        current_if_block = []
-                    current_if_block.append(line)
+                    # If a previous if-block is already open (no blank line or
+                    # comment separated it from this one), close it first.
+                    # Otherwise this new block's "then:" would be treated as a
+                    # continuation of the previous one's, and sorting would
+                    # pool both blocks' items together and redistribute them
+                    # across the block boundary.
+                    if current_if_block is not None and any(
+                        RE_IF_BLOCK.match(bl) for bl in current_if_block
+                    ):
+                        if_blocks.append(current_if_block)
+                        current_if_block = [line]
+                    else:
+                        if current_if_block is None:
+                            current_if_block = []
+                        current_if_block.append(line)
                     i += 1
                     continue
 
diff --git a/vinca/test_sort_vinca_lists.py b/vinca/test_sort_vinca_lists.py
new file mode 100644
index 0000000..44777c6
--- /dev/null
+++ b/vinca/test_sort_vinca_lists.py
@@ -0,0 +1,98 @@
+"""Tests for the vinca.yaml list sorter."""
+
+from vinca.sort_vinca_lists import sort_vinca_lists
+
+BASE = """packages_select_by_deps:
+  - alpha
+  - charlie
+  - bravo
+"""
+
+
+def test_sorts_simple_top_level_items(tmp_path):
+    path = tmp_path / "vinca.yaml"
+    path.write_text(BASE)
+
+    changed = sort_vinca_lists(path)
+
+    assert changed is True
+    items = [
+        line.strip()[2:]
+        for line in path.read_text().splitlines()
+        if line.startswith("  - ")
+    ]
+    assert items == ["alpha", "bravo", "charlie"]
+
+
+def test_leaves_already_sorted_file_unchanged(tmp_path):
+    path = tmp_path / "vinca.yaml"
+    sorted_content = "packages_select_by_deps:\n  - alpha\n  - bravo\n  - charlie\n\n"
+    path.write_text(sorted_content)
+
+    changed = sort_vinca_lists(path)
+
+    assert changed is False
+    assert path.read_text() == sorted_content
+
+
+def test_adjacent_if_blocks_without_separator_stay_isolated(tmp_path):
+    # Regression test: two "- if:" blocks back-to-back with no blank line or
+    # comment between them used to be parsed as a single block, so sorting
+    # pooled both blocks' then-items together and could redistribute an item
+    # from one block's platform condition into the other's.
+    content = """packages_select_by_deps:
+  - if: not wasm32 and not win
+    then:
+      - web_video_server
+      - webots_ros2
+      - yasmin
+      - yasmin_ros
+  - if: linux and not aarch64
+    then:
+      - somepkg
+      - zed_msgs
+
+patch_dir: patch
+"""
+    path = tmp_path / "vinca.yaml"
+    path.write_text(content)
+
+    sort_vinca_lists(path)
+
+    result = path.read_text()
+    first_block, second_block = result.split("- if: not wasm32 and not win")[1].split(
+        "- if: linux and not aarch64"
+    )
+
+    assert "webots_ros2" in first_block
+    assert "yasmin_ros" in first_block
+    assert "webots_ros2" not in second_block
+    assert "somepkg" in second_block
+    assert "zed_msgs" in second_block
+
+
+def test_adjacent_if_blocks_separated_by_comment_stay_isolated(tmp_path):
+    # The comment-separator workaround must keep working alongside the fix.
+    content = """packages_select_by_deps:
+  - if: not wasm32 and not win
+    then:
+      - web_video_server
+      - yasmin
+      - yasmin_ros
+
+  # webots_ros2 is linux-only
+  - if: linux and not aarch64
+    then:
+      - webots_ros2
+      - zed_msgs
+
+patch_dir: patch
+"""
+    path = tmp_path / "vinca.yaml"
+    path.write_text(content)
+
+    sort_vinca_lists(path)
+
+    first_block, second_block = path.read_text().split("- if: linux and not aarch64")
+    assert "- webots_ros2" not in first_block
+    assert "- webots_ros2" in second_block