Review fixes

.claude/skills/plugin-development/SKILL.md

@@ -65,7 +65,7 @@ Every mapped field (`objectPrimaryId`/`objectSecondaryId`/`watchedValue1-4`/`ext
 
 `all_plugins` (every plugin's parsed `config.json`) is loaded once, in `initialise.py`'s `get_plugins_configs()` call, and reused as the same list object everywhere after that until an actual config reload replaces it. `update_api()` (`server/api.py`), which receives `all_plugins` as a parameter, runs far more often than that - on every scan cycle and every plugin completion (`server/__main__.py`, `server/plugin.py`), not just on reload.
 
-Any function deriving something from `all_plugins` that gets called from a hot path like `update_api()` should cache its result keyed by `id(all_plugins)`, not recompute on every call - the input is almost always the same object even though the call site fires constantly. `server/db/plugin_field_views.py`'s `get_all_device_field_views()` is the pattern: check `id(all_plugins)` against the last-seen id, recomputing only when a genuinely new object (an actual reload) appears.
+Any function deriving something from `all_plugins` that gets called from a hot path like `update_api()` should cache its result, not recompute on every call - the input is almost always the same object even though the call site fires constantly. `server/db/plugin_field_views.py`'s `get_all_device_field_views()` is the pattern: retain a reference to the last-seen `all_plugins` list and compare it to the current one with `is`, recomputing only when a genuinely new object (an actual reload) appears. Don't key the cache by `id(all_plugins)` alone - once the old list is garbage collected, Python can reuse that id for an unrelated object, which would wrongly serve a stale cached result for what is actually a new reload.
 
 ## Plugin Formats
 

.gemini/skills/plugin-development/plugin-skill.md

@@ -77,7 +77,7 @@ Every mapped field (`objectPrimaryId`/`objectSecondaryId`/`watchedValue1-4`/`ext
 
 `all_plugins` is loaded once (`initialise.py`'s `get_plugins_configs()`) and reused as the same object until a reload - but `update_api()` (`server/api.py`), which takes it as a parameter, runs on every scan cycle and plugin completion, not just on reload.
 
-- If you write a function that derives something from `all_plugins` and gets called from `update_api()` or another frequent call site, cache the result by `id(all_plugins)` instead of recomputing every call
+- If you write a function that derives something from `all_plugins` and gets called from `update_api()` or another frequent call site, cache the result instead of recomputing every call - retain a reference to the last-seen `all_plugins` list and compare it to the current one with `is`, not by `id(all_plugins)` alone (once the old list is garbage collected, Python can reuse that id for an unrelated object, wrongly serving a stale cached result for what is actually a new reload)
 - Pattern: `server/db/plugin_field_views.py`'s `get_all_device_field_views()`
 
 ## Plugin Formats

.github/skills/plugin-run-development/SKILL.md

@@ -78,7 +78,7 @@ Every mapped field (`objectPrimaryId`/`objectSecondaryId`/`watchedValue1-4`/`ext
 
 - `all_plugins` is loaded once (`get_plugins_configs()` in `initialise.py`) and reused as the same object until a config reload
 - `update_api()` (`server/api.py`) receives `all_plugins` but runs on every scan cycle and plugin completion - far more often than reloads happen
-- A function deriving something from `all_plugins`, called from a hot path like `update_api()`, should cache by `id(all_plugins)` rather than recompute every call
+- A function deriving something from `all_plugins`, called from a hot path like `update_api()`, should cache its result rather than recompute every call - retain a reference to the last-seen `all_plugins` list and compare it to the current one with `is`, not by `id(all_plugins)` alone (once the old list is garbage collected, Python can reuse that id for an unrelated object, wrongly serving a stale cached result for what is actually a new reload)
 - Example: `server/db/plugin_field_views.py`'s `get_all_device_field_views()`
 
 ## Plugin Formats

server/db/plugin_field_views.py

@@ -90,7 +90,7 @@ def get_plugin_columns_for_field(all_plugins, field_key):
     return results
 
 
-_field_views_cache = {"all_plugins_id": None, "result": None}
+_field_views_cache = {"all_plugins_ref": None, "result": None}
 
 
 def get_all_device_field_views(all_plugins):
@@ -99,14 +99,16 @@ def get_all_device_field_views(all_plugins):
     reload and reused as the same object across every later call (e.g. update_api() runs on
     every scan cycle and plugin completion) - caching by identity avoids re-scanning every
     plugin's config on each of those calls, recomputing only when a genuinely new all_plugins
-    object (an actual reload) is passed in."""
-    cache_key = id(all_plugins)
-    if _field_views_cache["all_plugins_id"] != cache_key:
+    object (an actual reload) is passed in. The cache retains a reference to the list itself
+    (compared with `is`), not just its id() - an id() alone can be reused by an unrelated
+    object once the original all_plugins list is garbage collected, which would wrongly
+    serve a stale cached result for what is actually a new reload."""
+    if _field_views_cache["all_plugins_ref"] is not all_plugins:
         _field_views_cache["result"] = {
             field_key: get_plugin_columns_for_field(all_plugins, field_key)
             for field_key in DEVICE_FIELD_VIEWS
         }
-        _field_views_cache["all_plugins_id"] = cache_key
+        _field_views_cache["all_plugins_ref"] = all_plugins
     return _field_views_cache["result"]
 
 

test/backend/test_app_conf_string_escaping.py

@@ -25,13 +25,19 @@ pytestmark = pytest.mark.skipif(PHP_BIN is None, reason="PHP CLI (php or php83)
 
 # Stands in for the web request: reads {"front": ..., "settings": [...]} from stdin,
 # satisfies security.php's request-only dependencies and lets util.php dispatch savesettings.
+# util.php only reads function/settings from $_POST (not $_REQUEST) and requires a matching
+# csrf_token - seed $_SESSION directly (same PHP process, so security.php's session_start()
+# just resumes it) rather than $_REQUEST, to mirror a real authenticated POST.
 PHP_RUNNER = (
     '$in = json_decode(stream_get_contents(STDIN), true);'
     'if (!function_exists("apache_request_headers")) { function apache_request_headers() { return []; } }'
     '$_SERVER["DOCUMENT_ROOT"] = $in["front"];'
     '$_SERVER["HTTP_HOST"] = "localhost";'
     '$_SERVER["REQUEST_URI"] = "/php/server/util.php";'
-    '$_REQUEST = ["function" => "savesettings", "settings" => json_encode($in["settings"])];'
+    'session_start();'
+    '$_SESSION["csrf_token"] = "test-harness-csrf-token";'
+    '$_POST = ["function" => "savesettings", "csrf_token" => "test-harness-csrf-token", '
+    '"settings" => json_encode($in["settings"])];'
     'require $in["front"] . "/php/server/util.php";'
 )