Protect action identity from mutation

Keep action identity and display metadata
constructor-set so registry keys cannot drift
from the objects they index.

Require sources to publish unique ownership while
the registry retains shared keepalive for runs.

Preserve opaque IDs and persisted state keys.

src/slic3r/GUI/ActionRegistry.cpp

@@ -68,15 +68,16 @@ void ActionRegistry::add_source(std::unique_ptr<IActionSource> source)
         m_sources.push_back(std::move(source));
 }
 
-void ActionRegistry::upsert(std::shared_ptr<AppAction> action)
+void ActionRegistry::upsert(std::unique_ptr<AppAction> action)
 {
     assert(wxThread::IsMain());
     if (!action)
         return;
 
     seed_state(*action);
-    std::string id = action->id;
-    m_actions.insert_or_assign(std::move(id), std::move(action));
+    std::string id = action->id();
+    std::shared_ptr<AppAction> stored = std::move(action);
+    m_actions.insert_or_assign(std::move(id), std::move(stored));
 }
 
 void ActionRegistry::remove(const std::string& id)
@@ -88,10 +89,10 @@ void ActionRegistry::remove(const std::string& id)
 void ActionRegistry::seed_state(AppAction& a) const
 {
     auto favs = read_string_array("favourite_actions");
-    a.favourite = std::find(favs.begin(), favs.end(), a.id) != favs.end();
+    a.favourite = std::find(favs.begin(), favs.end(), a.id()) != favs.end();
 
     nlohmann::json stats = read_section("stats", nlohmann::json::object());
-    auto it = stats.find(a.id);
+    auto it = stats.find(a.id());
     if (it != stats.end() && it->is_object()) {
         a.count = it->value("count", 0);
         a.last  = it->value("last", 0LL);
@@ -208,18 +209,18 @@ nlohmann::json ActionRegistry::snapshot() const
         double sb = frecency_score(b->count, b->last, now);
         if (sa != sb)
             return sa > sb;
-        if (a->title != b->title)
-            return a->title < b->title;
-        if (a->source != b->source)
-            return a->source < b->source;
-        return a->id < b->id;
+        if (a->title() != b->title())
+            return a->title() < b->title();
+        if (a->source() != b->source())
+            return a->source() < b->source();
+        return a->id() < b->id();
     });
 
     nlohmann::json actions = nlohmann::json::array();
     for (const AppAction* a : sorted)
-        actions.push_back({{"id", a->id},
-                           {"title", a->title},
-                           {"source", a->source},
+        actions.push_back({{"id", a->id()},
+                           {"title", a->title()},
+                           {"source", a->source()},
                            {"shortcut", ""}});
     // why: favourites is the ORDERED pin list - it must come from favourite_actions
     // as stored, not be re-derived from the frecency-sorted actions (that would

src/slic3r/GUI/ActionRegistry.hpp

@@ -33,11 +33,9 @@ struct AppActionRunResult
 // UnsavedChangesDialog's exit-action enum, and this header reaches most GUI TUs.
 struct AppAction
 {
-    std::string id;           // speed_dial_action_id(...) - stable identity + AppConfig key
-    std::string title;        // display name
-    // Where the action comes from. Each subclass sets it in its constructor, so the
-    // base cannot be built without one.
-    std::string source;
+    const std::string& id() const { return m_id; }
+    const std::string& title() const { return m_title; }
+    const std::string& source() const { return m_source; }
 
     // seeded from AppConfig for the snapshot / sort:
     bool        favourite = false;
@@ -48,9 +46,15 @@ struct AppAction
     virtual AppActionRunResult run() const = 0; // re-resolves + runs (UI thread)
 
 protected:
-    // why: protected + no default forces every subclass to supply id/title/source.
+    // The definition is constructor-set and immutable. Refreshes replace an action
+    // instead of mutating identity after the registry has indexed it by id.
     AppAction(std::string id, std::string title, std::string source)
-        : id(std::move(id)), title(std::move(title)), source(std::move(source)) {}
+        : m_id(std::move(id)), m_title(std::move(title)), m_source(std::move(source)) {}
+
+private:
+    std::string m_id;       // speed_dial_action_id(...) - stable identity + AppConfig key
+    std::string m_title;    // display name
+    std::string m_source;   // display name of the action's source
 };
 
 // Generic sink and single owner of runnable actions for the app session.
@@ -74,9 +78,9 @@ public:
     // until init() starts it.
     void add_source(std::unique_ptr<IActionSource> source);
 
-    // Seeds persisted state, then inserts the action or replaces the action with the
-    // same id. A null action is ignored.
-    void upsert(std::shared_ptr<AppAction> action);
+    // Takes ownership, seeds persisted state, then inserts the action or replaces
+    // the action with the same id. A null action is ignored.
+    void upsert(std::unique_ptr<AppAction> action);
 
     // Removes the action with this id. Missing ids are a harmless no-op.
     void remove(const std::string& id);

src/slic3r/GUI/ScriptActionSource.cpp

@@ -103,15 +103,15 @@ void ScriptActionSource::start(ActionRegistry& sink)
         auto source = source_names.find(capability->plugin_key);
         const std::string& source_name = source == source_names.end() ? capability->plugin_key : source->second;
         if (auto action = make_action(capability->plugin_key, capability->name, source_name)) {
-            track(capability->plugin_key, action->id);
+            track(capability->plugin_key, action->id());
             m_sink->upsert(std::move(action));
         }
     }
 }
 
-std::shared_ptr<AppAction> ScriptActionSource::make_action(const std::string& plugin_key,
-                                                            const std::string& capability,
-                                                            const std::string& source) const
+std::unique_ptr<AppAction> ScriptActionSource::make_action(const std::string& plugin_key,
+                                                           const std::string& capability,
+                                                           const std::string& source) const
 {
     PluginLoader& loader = PluginManager::instance().get_loader();
     if (!loader.is_plugin_loaded(plugin_key))
@@ -121,7 +121,7 @@ std::shared_ptr<AppAction> ScriptActionSource::make_action(const std::string& pl
     if (!loaded || !loaded->enabled)
         return nullptr;
 
-    return std::make_shared<PluginScriptAction>(plugin_key, capability, source);
+    return std::make_unique<PluginScriptAction>(plugin_key, capability, source);
 }
 
 void ScriptActionSource::refresh_capability(const std::string& plugin_key, const std::string& capability,
@@ -169,7 +169,7 @@ void ScriptActionSource::refresh_source(const std::string& plugin_key, ActionCha
         if (!capability || !capability->enabled)
             continue;
         if (auto action = make_action(plugin_key, capability->name, source_name)) {
-            track(plugin_key, action->id);
+            track(plugin_key, action->id());
             m_sink->upsert(std::move(action));
         }
     }

src/slic3r/GUI/ScriptActionSource.hpp

@@ -28,7 +28,7 @@ public:
 private:
     void refresh_source(const std::string& plugin_key, ActionChange change);
     void refresh_capability(const std::string& plugin_key, const std::string& capability, ActionChange change);
-    std::shared_ptr<AppAction> make_action(const std::string& plugin_key, const std::string& capability,
+    std::unique_ptr<AppAction> make_action(const std::string& plugin_key, const std::string& capability,
                                            const std::string& source) const;
     void track(const std::string& plugin_key, const std::string& id);
     void untrack(const std::string& plugin_key, const std::string& id);

src/slic3r/GUI/SpeedDialDialog.cpp

@@ -255,7 +255,7 @@ private:
         if (!a)
             return;
         const bool        ask    = reg.should_ask(id);
-        const std::string atitle = a->title;   // capture before Dismiss; `a` may not outlive it
+        const std::string atitle = a->title(); // capture before Dismiss; `a` may not outlive it
         Dismiss(); // launcher gone before the modal / the script's own UI
 
         if (ask) {

tests/slic3rutils/test_action_source.cpp

@@ -4,7 +4,11 @@
 #include "slic3r/GUI/IActionSource.hpp"
 
 #include <memory>
+#include <string>
+#include <type_traits>
 
+using Slic3r::GUI::AppAction;
+using Slic3r::GUI::AppActionRunResult;
 using Slic3r::GUI::ActionRegistry;
 using Slic3r::GUI::IActionSource;
 
@@ -21,8 +25,39 @@ private:
     bool& m_started;
 };
 
+// AppAction is abstract; this minimal concrete action lets the tests exercise
+// its constructor-set definition without involving a plugin runner.
+class TestAppAction final : public AppAction
+{
+public:
+    TestAppAction() : AppAction("action-id", "Action title", "Action source") {}
+
+    AppActionRunResult run() const override { return {}; }
+};
+
 } // namespace
 
+TEST_CASE("AppAction definitions are immutable after construction", "[speeddial][actions]")
+{
+    using StringAccessor = const std::string& (AppAction::*)() const;
+
+    STATIC_CHECK(std::is_same_v<decltype(&AppAction::id), StringAccessor>);
+    STATIC_CHECK(std::is_same_v<decltype(&AppAction::title), StringAccessor>);
+    STATIC_CHECK(std::is_same_v<decltype(&AppAction::source), StringAccessor>);
+
+    const TestAppAction action;
+    CHECK(action.id() == "action-id");
+    CHECK(action.title() == "Action title");
+    CHECK(action.source() == "Action source");
+}
+
+TEST_CASE("ActionRegistry takes exclusive ownership of published actions", "[speeddial][actions]")
+{
+    using ExpectedUpsert = void (ActionRegistry::*)(std::unique_ptr<AppAction>);
+
+    STATIC_CHECK(std::is_same_v<decltype(&ActionRegistry::upsert), ExpectedUpsert>);
+}
+
 TEST_CASE("ActionRegistry starts registered action sources", "[speeddial][actions]")
 {
     bool started = false;