From af85c9df20c96a608dc2814bc1ac414941e8da39 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jean-S=C3=A9bastien=20Bevilacqua?=
 <jean-sebastien.bevilacqua@6wind.com>
Date: Wed, 4 Mar 2026 16:06:42 +0100
Subject: [PATCH] change: fix update_config_cache crash when 'after' entry is
 missing
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When sysrepo sends a batch of changes where a ChangeDeleted removes a
list entry and a subsequent ChangeCreated references that same entry in
its 'after' parameter (to specify insertion order), xpath_set() raises a
ValueError because the referenced entry no longer exists in the cache.

This happens in practice when list entries are rapidly created and
deleted (e.g. coredump files appearing during repeated service crashes).
The ValueError propagates up and kills the sysrepo subscription callback,
which in the case of the alarm manager means no further alarms are
processed for the rest of the session.

Fix this by catching the ValueError and retrying without the 'after'
parameter, which appends the new entry at the end of the list. The entry
is still created, only the insertion order is lost, which is an
acceptable trade-off compared to crashing the entire callback.

The fix is done in sysrepo-python (the caller that knows the context)
rather than in libyang-python (which is correct to be strict about
invalid 'after' references).

Add a dedicated unit test for update_config_cache covering both the
normal case (after entry exists) and the fallback case (after entry
was deleted).

Signed-off-by: Jean-Sébastien Bevilacqua <jean-sebastien.bevilacqua@6wind.com>
---
 sysrepo/change.py                 |  7 +++-
 tests/test_update_config_cache.py | 56 +++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 1 deletion(-)
 create mode 100644 tests/test_update_config_cache.py

diff --git a/sysrepo/change.py b/sysrepo/change.py
index a0f3885..70288c4 100644
--- a/sysrepo/change.py
+++ b/sysrepo/change.py
@@ -206,7 +206,12 @@ def update_config_cache(conf: Dict, changes: List[Change]) -> None:
     """
     for c in changes:
         if isinstance(c, ChangeCreated):
-            libyang.xpath_set(conf, c.xpath, c.value, after=c.after)
+            try:
+                libyang.xpath_set(conf, c.xpath, c.value, after=c.after)
+            except ValueError:
+                # The 'after' entry may have been deleted by a previous change
+                # in the same batch. Fall back to appending at the end.
+                libyang.xpath_set(conf, c.xpath, c.value)
         elif isinstance(c, ChangeModified):
             libyang.xpath_set(conf, c.xpath, c.value)
         elif isinstance(c, ChangeMoved):
diff --git a/tests/test_update_config_cache.py b/tests/test_update_config_cache.py
new file mode 100644
index 0000000..934d167
--- /dev/null
+++ b/tests/test_update_config_cache.py
@@ -0,0 +1,56 @@
+# Copyright (c) 2026 6WIND S.A.
+# SPDX-License-Identifier: BSD-3-Clause
+
+import unittest
+
+from sysrepo.change import ChangeCreated, ChangeDeleted, update_config_cache
+
+
+class UpdateConfigCacheTest(unittest.TestCase):
+    def test_created_after_missing_entry(self):
+        """
+        When a ChangeCreated references an 'after' entry that was deleted by a
+        previous ChangeDeleted in the same batch, the entry should still be
+        created (appended at the end) instead of raising ValueError.
+        """
+        conf = {
+            "coredump": [
+                {"name": "core.A"},
+                {"name": "core.B"},
+            ],
+        }
+        changes = [
+            ChangeDeleted("/coredump[name='core.A']", {"name": "core.A"}),
+            ChangeCreated(
+                "/coredump[name='core.C']",
+                {"name": "core.C"},
+                after="[name='core.A']",
+            ),
+        ]
+        update_config_cache(conf, changes)
+        # core.A deleted, core.C added (appended since 'after' ref is gone)
+        self.assertEqual(
+            conf["coredump"],
+            [{"name": "core.B"}, {"name": "core.C"}],
+        )
+
+    def test_created_after_existing_entry(self):
+        """Normal case: 'after' entry exists, insertion at correct position."""
+        conf = {
+            "coredump": [
+                {"name": "core.A"},
+                {"name": "core.B"},
+            ],
+        }
+        changes = [
+            ChangeCreated(
+                "/coredump[name='core.C']",
+                {"name": "core.C"},
+                after="[name='core.A']",
+            ),
+        ]
+        update_config_cache(conf, changes)
+        self.assertEqual(
+            conf["coredump"],
+            [{"name": "core.A"}, {"name": "core.C"}, {"name": "core.B"}],
+        )