fix: close the scheduled-message update check/write race

PUT verified original_schedule outside the lock, so a concurrent delete
between the check and the write resurrected the entry as a new one, and
two concurrent renames of the same original left both results present.
The existence check now runs inside _save_scheduled_message_locked
against the same snapshot the duplicate check uses, so create, update and
delete are each a single critical section.

Added a concurrency test: two simultaneous creates of one schedule now
produce exactly one 200 and one 409 with a single entry on disk.

modules/web_viewer/app.py

@@ -4045,6 +4045,15 @@ class BotDataViewer:
                 return jsonify({'success': False, 'error': described.get('error')}), 400
 
             existing = _existing_schedules()
+
+            # Checked here rather than in the route so it shares this snapshot and the
+            # surrounding lock.
+            if replacing is not None and replacing not in existing:
+                return jsonify({
+                    'success': False,
+                    'error': f"No scheduled message for '{replacing}'",
+                }), 404
+
             # Schedules are INI keys, so two entries cannot share one. Renaming onto
             # another entry's key would silently overwrite it.
             if schedule in existing and schedule != replacing:
@@ -4114,8 +4123,9 @@ class BotDataViewer:
                 original = (data.get('original_schedule') or '').strip()
                 if not original:
                     return jsonify({'success': False, 'error': 'original_schedule is required'}), 400
-                if original not in _existing_schedules():
-                    return jsonify({'success': False, 'error': f"No scheduled message for '{original}'"}), 404
+                # Existence is verified inside the lock, against the same snapshot the
+                # duplicate check uses: a concurrent delete between an outside check and
+                # the write would otherwise resurrect the entry as a new one.
                 return _save_scheduled_message(data, replacing=original)
             except Exception as e:
                 self.logger.error(f"Error updating scheduled message: {e}")

tests/test_scheduled_messages_api.py

@@ -204,3 +204,44 @@ class TestRoundTrip:
         raw = parser.get("Scheduled_Messages", "15 9 * * mon,fri")
         channel, message, scope = parse_scheduled_message_value(raw)
         assert (channel, message, scope) == ("Public", "Standup at 9:15", "#sea")
+
+
+class TestWriteSerialisation:
+    """Duplicate/existence checks must share the write's critical section, or a
+    concurrent request can slip between the check and the write."""
+
+    def test_concurrent_creates_of_one_schedule_yield_exactly_one_success(self, viewer):
+        import threading
+
+        results = []
+        barrier = threading.Barrier(2)
+
+        def create(body):
+            client = viewer.app.test_client()
+            barrier.wait()
+            resp = _post(client, "/api/scheduled-messages", body)
+            results.append(resp.status_code)
+
+        threads = [
+            threading.Thread(target=create, args=({
+                "schedule": "45 7 * * *", "channel": "Public", "message": f"msg {i}",
+            },)) for i in range(2)
+        ]
+        for t in threads:
+            t.start()
+        for t in threads:
+            t.join()
+
+        assert sorted(results) == [200, 409], results
+        # And exactly one entry landed, not one silently replacing the other.
+        assert _config_text(viewer).count("45 7 * * * =") == 1
+
+    def test_update_of_a_removed_entry_is_a_404_not_a_resurrection(self, viewer):
+        client = _client(viewer)
+        _post(client, "/api/scheduled-messages", {"schedule": "0 8 * * *"}, method="delete")
+        resp = _post(client, "/api/scheduled-messages", {
+            "original_schedule": "0 8 * * *", "schedule": "0 9 * * *",
+            "channel": "Public", "message": "back from the dead",
+        }, method="put")
+        assert resp.status_code == 404
+        assert "back from the dead" not in _config_text(viewer)