fix: honor can_be_disabled and declared defaults when loading agent action enabled flags (#174) (#181)

* fix: honor can_be_disabled and declared defaults when loading agent action enabled flags (#174)

* fix: surface refused agent action toggles, enforce enabled/can_be_disabled at the model, correct changelog claims (#174)

* fix: correct ToggleAgentAction docs wording and error message, restore test class placement (#174)

CHANGELOG.yaml

@@ -62,6 +62,8 @@
   - "Character Card Import: Attribute extraction no longer runs blind in the default (non-Fast) import mode — the character being profiled was missing from its own extraction prompt, so attributes were generated from the greeting and character book alone, without the character's description. Imported attributes now reflect the description."
   - "Character Creation: The director's 'Limit character attributes' setting now delivers the number of attributes it promises. The character's own name is written into the generated character sheet as a `Name` line, and it was counted against the limit — so every value delivered one attribute fewer than configured, and a limit of 1 produced no attributes at all. The name no longer costs a slot, in generated sheets as well as in sheets supplied to the Persist Character node."
   - "Fast Character Creation: The one-shot generation prompt no longer repeats the character's name as an attribute — the name is already generated as its own aspect, so a `Name` entry in the character sheet was duplication, and under a configured attribute limit it consumed one of the allowed lines. This applies at every value of the director's 'Limit character attributes' setting, including the default of 0: Fast mode no longer asks for a `Name` attribute, and one written anyway does not cost a slot."
+  - "Agent Settings: An agent setting that has no on/off switch is no longer left switched off by a stored configuration. Sections like the Summarizer's Summarization, the Conversation and Narrator Generation sections or the World State's Character Portraits are always on by design, so the settings screen shows their options without an Enable checkbox — but a `false` recorded in the configuration file was still applied on startup, switching the section off in the background while its options stayed on display, with no control anywhere to turn it back on. The declaration now wins: such a setting is always loaded as on, and a `false` left in the configuration file — or in a scene's per-scene overrides — is ignored. A node graph that tries to toggle one now reports an error instead of appearing to succeed."
+  - "Agent Settings: Newly added agent settings no longer arrive switched off on an existing installation. Any setting that did not yet exist when the configuration file was last written was loaded as off rather than in the state it ships with, and then recorded as off — so a feature added in a release could be silently inactive for everyone upgrading into it while a fresh installation got it working. It is also what wrote the stale `false` the fix above corrects. Settings absent from the stored configuration now keep the state they ship with; a setting you switched off yourself stays off."
   - "Example Dialogue: Character example dialogue no longer keeps typographic quotes (“ ” „ ‘ ’ ‚). Talemate delimits spoken words with the straight quote, so an example line carrying fancy quotes rendered as narration instead of dialogue and was mis-chunked by dialogue parsing and text-to-speech. Typographic quotes are now replaced with their plain equivalents wherever example dialogue is written outside a generation — character card import, manual entry and edits in the world editor, examples supplied directly to the Persist Character node, and the creator's example dialogue generation. Generated text was already normalized at the client level, and now also covers the German-style low-9 opening quotes, so a `„…“` pair no longer collapses into a single unbalanced straight quote. Characters already saved with fancy quotes keep them until the affected line is saved again."
   - "Help Agent: Documentation lookups now find the page that actually answers the question. Every word counted the same, so asking about 'koboldcpp settings' returned five different agent settings pages and no KoboldCpp page at all — the common word decided the match and the rest were ties broken alphabetically. Distinctive words now count for far more than ubiquitous ones. Page length also buys far less rank: a longer description of a page used to be a strictly better one, every extra word another free chance to match, so describing a page more thoroughly made it surface for topics it only mentions in passing. Those matches are now diluted by how much the description covers, so a page that mentions a topic in passing no longer outranks the page about it."
 

docs/user-guide/node-editor/reference/nodes/agents.md

@@ -249,9 +249,11 @@ Provides a required `state` input causing the node to only run when a state is p
 
 `agents/ToggleAgentAction`
 
-Allows disabling or enabling an agent action
+Allows disabling or enabling an agent action that can be disabled
 
-Raises an error if the agent or the action cannot be found.
+Raises an error if the agent or the action cannot be found, or if the
+action is one that cannot be disabled — those are always enabled and
+are not togglable from a graph.
 
 **Inputs**
 
@@ -269,7 +271,7 @@ Raises an error if the agent or the action cannot be found.
 | `state` | `any` | The state input, passed through |
 | `agent` | `agent` | The resolved agent instance |
 | `action_name` | `str` | The action name, passed through |
-| `enabled` | `bool` | The enabled state that was set |
+| `enabled` | `bool` | The action's effective enabled state after the write — when a scene override is active it takes the write, so this reflects the override rather than the agent's global setting |
 
 **Properties**
 

src/talemate/agents/base.py

@@ -119,6 +119,22 @@ class AgentAction(pydantic.BaseModel):
     # toggled at all is overridable per scene unless it opts out explicitly.
     enabled_scene_overridable: bool | None = None
 
+    @pydantic.model_validator(mode="after")
+    def _disabled_requires_can_be_disabled(self):
+        # An action nothing can turn on that ships turned off is unreachable:
+        # the global UI renders no Enable checkbox without can_be_disabled,
+        # and `resolve_enabled` reports it on regardless. Rejecting the
+        # combination at construction keeps the declaration and the resolver
+        # from disagreeing — including for dynamically synthesized children,
+        # which the shipped-action sweep in the tests cannot see.
+        if not self.enabled and not self.can_be_disabled:
+            raise ValueError(
+                f"AgentAction {self.label!r}: enabled=False requires "
+                "can_be_disabled=True — an action that cannot be disabled "
+                "always resolves as enabled"
+            )
+        return self
+
     @pydantic.model_validator(mode="after")
     def _enabled_scene_overridable_requires_can_be_disabled(self):
         # An enable-flag override only makes sense when the global enable
@@ -878,10 +894,18 @@ class Agent(ABC):
 
     def resolve_enabled(self, action_key: str) -> bool:
         """Return the effective enabled flag for a container action."""
+        action = self.actions[action_key]
+
+        # The declaration wins: an action without can_be_disabled has no
+        # Enable control anywhere in the UI, so an override that turns it off
+        # (hand-edited or written by an older version) could never be undone.
+        if not action.can_be_disabled:
+            return True
+
         return bool(
             self._resolve(
                 lambda o: o.get_enabled(self.agent_type, action_key),
-                lambda: self.actions[action_key].enabled,
+                lambda: action.enabled,
             )
         )
 
@@ -913,6 +937,18 @@ class Agent(ABC):
 
         Note: this updates an *existing* override; it does not create a new one.
         """
+        # Same rule as resolve_enabled / apply_config: the declaration wins.
+        # Writing the flag here would otherwise persist a value the resolver
+        # ignores, leaving the stored config disagreeing with what runs.
+        if not self.actions[action_key].can_be_disabled:
+            log.warning(
+                "write_enabled refused: action cannot be disabled",
+                agent=self.agent_type,
+                action=action_key,
+                requested=enabled,
+            )
+            return
+
         self._route_write(
             lambda o: o.get_enabled(self.agent_type, action_key) is not UNSET,
             lambda o: o.set_enabled(self.agent_type, action_key, enabled),
@@ -949,9 +985,20 @@ class Agent(ABC):
             if not kwargs.get("actions"):
                 continue
 
-            action.enabled = (
-                kwargs.get("actions", {}).get(action_key, {}).get("enabled", False)
-            )
+            if not action.can_be_disabled:
+                # See resolve_enabled: nothing in the UI can turn these back
+                # on, so a saved `enabled: false` (written by an older version
+                # or hand-edited) is discarded rather than honored.
+                action.enabled = True
+            else:
+                # Falling back to the action's current value rather than False
+                # keeps an action the saved config predates in the state it
+                # ships with, instead of silently disabling it on first load.
+                action.enabled = (
+                    kwargs.get("actions", {})
+                    .get(action_key, {})
+                    .get("enabled", action.enabled)
+                )
 
             if not action.config:
                 continue

src/talemate/game/engine/nodes/agent.py

@@ -152,9 +152,11 @@ class AgentSettingsNode(Node):
 @register("agents/ToggleAgentAction")
 class ToggleAgentAction(Node):
     """
-    Allows disabling or enabling an agent action
+    Allows disabling or enabling an agent action that can be disabled
 
-    Raises an error if the agent or the action cannot be found.
+    Raises an error if the agent or the action cannot be found, or if the
+    action is one that cannot be disabled — those are always enabled and
+    are not togglable from a graph.
 
     Inputs:
 
@@ -168,7 +170,9 @@ class ToggleAgentAction(Node):
     - state: The state input, passed through
     - agent: The resolved agent instance
     - action_name: The action name, passed through
-    - enabled: The enabled state that was set
+    - enabled: The action's effective enabled state after the write — when a
+      scene override is active it takes the write, so this reflects the
+      override rather than the agent's global setting
     """
 
     class Fields:
@@ -229,7 +233,18 @@ class ToggleAgentAction(Node):
             raise InputValueError(
                 self,
                 "action_name",
-                f"Could not find action {action_name} in agent {agent}",
+                f"Could not find action {action_name} in agent {agent.agent_type}",
+            )
+
+        if not action.can_be_disabled:
+            # The write would be refused, so say so rather than pass the graph
+            # through as if the action had been toggled. Matches how this node
+            # already reports an unknown agent or action.
+            raise InputValueError(
+                self,
+                "action_name",
+                f"Action {action_name} on agent {agent.agent_type} is always "
+                "enabled and cannot be toggled",
             )
 
         agent.write_enabled(action_name, enabled)
@@ -239,7 +254,10 @@ class ToggleAgentAction(Node):
                 "state": self.get_input_value("state"),
                 "agent": agent,
                 "action_name": action_name,
-                "enabled": enabled,
+                # Read back through the resolver rather than echoing the input:
+                # when a scene override is active it takes the write, so this
+                # reflects the override rather than the agent's global setting.
+                "enabled": agent.resolve_enabled(action_name),
             }
         )
 

talemate_frontend/src/constants/sceneAgentSettings.js

@@ -111,6 +111,9 @@ export function configOverrideActive(overlay, actionKey, configKey) {
  * active, the agent's global flag otherwise. Mirrors `Agent.resolve_enabled`.
  */
 export function effectiveActionEnabled(actions, overlay, actionKey) {
+  // An action without can_be_disabled has no enable control anywhere, so the
+  // declaration wins over any stored flag — matches Agent.resolve_enabled.
+  if (actions?.[actionKey] && !actions[actionKey].can_be_disabled) return true;
   if (enabledOverrideActive(overlay, actionKey)) {
     return !!overlay.actions[actionKey].enabled;
   }

tests/test_agents_base.py

@@ -62,6 +62,7 @@ class _ToggleableAgent(Agent):
             "main": AgentAction(
                 enabled=True,
                 label="Main",
+                can_be_disabled=True,
                 config={
                     "field": AgentActionConfig(
                         type="text", label="field", value="orig"
@@ -192,6 +193,33 @@ class TestAgentActionEnabledSceneOverridable:
         assert len(toggleable) >= 20
 
 
+# ---------------------------------------------------------------------------
+# AgentAction.enabled vs can_be_disabled
+# ---------------------------------------------------------------------------
+
+
+class TestAgentActionDisabledRequiresCanBeDisabled:
+    """`enabled=False` without `can_be_disabled` is unreachable by construction.
+
+    `resolve_enabled` reports such an action on regardless, so the declaration
+    would disagree with the resolver — and with the direct `.enabled` readers
+    (`save_config`, the help agent's action payload).
+    """
+
+    def test_declaring_disabled_without_can_be_disabled_raises(self):
+        with pytest.raises(ValueError, match="enabled=False requires"):
+            AgentAction(label="Unreachable", enabled=False)
+
+    def test_disabled_is_allowed_when_disableable(self):
+        action = AgentAction(label="Opt In", enabled=False, can_be_disabled=True)
+        assert action.enabled is False
+
+    def test_enabled_and_non_disableable_is_the_normal_case(self):
+        action = AgentAction(label="Always on")
+        assert action.enabled is True
+        assert action.can_be_disabled is False
+
+
 # ---------------------------------------------------------------------------
 # args_and_kwargs_to_dict
 # ---------------------------------------------------------------------------
@@ -466,6 +494,110 @@ class TestApplyConfig:
         assert a.actions["main"].config["field"].value == original
 
 
+class TestApplyConfigEnabledValidation:
+    """`enabled` loaded from config.yaml is validated against the declaration.
+
+    Regression coverage for issue #174: a saved `enabled: false` disabled an
+    action declared `can_be_disabled=False`, which the UI exposes no control
+    to turn back on. The value got there because an action missing from the
+    saved config was disabled outright and then persisted by `save_config`.
+    """
+
+    def _agent(self) -> _MinimalAgent:
+        return _MinimalAgent(
+            actions={
+                "locked": AgentAction(
+                    enabled=True,
+                    label="Locked",
+                    container=True,
+                    can_be_disabled=False,
+                    config={
+                        "field": AgentActionConfig(
+                            type="number", label="field", value=1
+                        )
+                    },
+                ),
+                "toggleable": AgentAction(
+                    enabled=True,
+                    label="Toggleable",
+                    can_be_disabled=True,
+                ),
+            }
+        )
+
+    @pytest.mark.asyncio
+    async def test_stale_disabled_flag_is_discarded_when_not_disableable(self):
+        a = self._agent()
+        await a.apply_config(
+            actions={
+                "locked": {"enabled": False, "config": {"field": {"value": 5}}},
+                "toggleable": {"enabled": True},
+            }
+        )
+        assert a.actions["locked"].enabled is True
+        # The rest of the saved action is still applied.
+        assert a.actions["locked"].config["field"].value == 5
+
+    @pytest.mark.asyncio
+    async def test_saved_disabled_flag_is_honored_when_disableable(self):
+        a = self._agent()
+        await a.apply_config(
+            actions={
+                "locked": {"enabled": True},
+                "toggleable": {"enabled": False},
+            }
+        )
+        assert a.actions["toggleable"].enabled is False
+
+    @pytest.mark.asyncio
+    async def test_action_absent_from_saved_config_keeps_declared_default(self):
+        """A config predating the action must not silently disable it."""
+        a = self._agent()
+        await a.apply_config(actions={"unrelated": {"enabled": True}})
+        assert a.actions["locked"].enabled is True
+        assert a.actions["toggleable"].enabled is True
+
+    @pytest.mark.asyncio
+    async def test_declared_disabled_action_stays_disabled_when_absent(self):
+        """The absent-key fallback is the declaration, not an unconditional True."""
+        a = _MinimalAgent(
+            actions={
+                "opt_in": AgentAction(
+                    enabled=False, label="Opt In", can_be_disabled=True
+                ),
+            }
+        )
+        await a.apply_config(actions={"unrelated": {"enabled": True}})
+        assert a.actions["opt_in"].enabled is False
+
+    def test_no_shipped_action_declares_disabled_and_non_disableable(self):
+        """Cheap sweep of the shipped declarations.
+
+        `AgentAction` rejects this combination at construction, so this is a
+        belt-and-braces check that every shipped agent's `init_actions()`
+        actually builds — it is not the primary guard.
+
+        Forcing `enabled=True` for every non-disableable action is only
+        correct while no shipped action declares the contradictory
+        combination of `enabled=False` and `can_be_disabled=False` — such an
+        action would be permanently off with no way to reach it.
+        """
+        offenders = []
+        non_disableable = []
+        for agent_type, agent_cls in agents_module.AGENT_CLASSES.items():
+            for action_key, action in agent_cls.init_actions().items():
+                if action.can_be_disabled:
+                    continue
+                non_disableable.append(f"{agent_type}.{action_key}")
+                if not action.enabled:
+                    offenders.append(f"{agent_type}.{action_key}")
+
+        assert not offenders
+        # Guards against a vacuous pass if init_actions ever stops returning
+        # the shipped set.
+        assert len(non_disableable) >= 20
+
+
 # ---------------------------------------------------------------------------
 # Agent.on_game_loop_start (resets scene-scoped configs)
 # ---------------------------------------------------------------------------

tests/test_conversation_agent.py

@@ -124,15 +124,17 @@ class TestActionRegistration:
         _, conversation, _ = conversation_scene
         assert conversation.conversation_format == "movie_script"
 
-    def test_format_returns_movie_script_when_override_disabled(
-        self, conversation_scene
-    ):
+    def test_format_ignores_a_disabled_flag_on_the_override(self, conversation_scene):
+        # `generation_override` is declared can_be_disabled=False, so nothing
+        # in the UI can switch it off and `resolve_enabled` reports it on
+        # regardless of a stored flag (issue #174). The configured format
+        # therefore still applies — the movie_script fallback in
+        # `conversation_format` is unreachable for a healthy config.
         _, conversation, _ = conversation_scene
-        # Override config value first, then disable. Should fall back to
-        # the movie_script default.
+        assert conversation.actions["generation_override"].can_be_disabled is False
         conversation.actions["generation_override"].config["format"].value = "chat"
         conversation.actions["generation_override"].enabled = False
-        assert conversation.conversation_format == "movie_script"
+        assert conversation.conversation_format == "chat"
 
     def test_format_returns_value_when_override_enabled(self, conversation_scene):
         _, conversation, _ = conversation_scene

tests/test_nodes_agent.py

@@ -0,0 +1,105 @@
+"""Unit tests for talemate.game.engine.nodes.agent.
+
+Focused on `ToggleAgentAction`, whose contract depends on whether the target
+action declares `can_be_disabled` (issue #174): a disableable action is
+toggled and reports the effective value, a non-disableable one is refused
+outright rather than silently doing nothing.
+"""
+
+from __future__ import annotations
+
+import pytest
+
+from _node_test_helpers import run_node
+
+from talemate.agents.base import Agent, AgentAction
+from talemate.game.engine.nodes.agent import ToggleAgentAction
+from talemate.game.engine.nodes.core import InputValueError
+from talemate.scene_agent_settings import SceneAgentSettings
+from talemate.tale_mate import Scene
+
+
+class _TogglableAgent(Agent):
+    """Real Agent subclass carrying one action of each kind."""
+
+    agent_type = "toggle-node-test"
+    verbose_name = "Toggle Node Test"
+    requires_llm_client = False
+
+    def __init__(self, scene=None):
+        self.actions = {
+            "toggleable": AgentAction(
+                enabled=True,
+                label="Toggleable",
+                container=True,
+                can_be_disabled=True,
+            ),
+            "locked": AgentAction(
+                enabled=True,
+                label="Locked",
+                container=True,
+                can_be_disabled=False,
+            ),
+        }
+        self.scene = scene
+        self.processing = 0
+
+
+@pytest.mark.asyncio
+async def test_toggles_a_disableable_action_and_reports_it():
+    agent = _TogglableAgent()
+    outputs = await run_node(
+        ToggleAgentAction(),
+        inputs={"agent": agent, "action_name": "toggleable", "enabled": False},
+    )
+    assert agent.actions["toggleable"].enabled is False
+    assert outputs["enabled"] is False
+    assert outputs["action_name"] == "toggleable"
+
+
+@pytest.mark.asyncio
+async def test_refuses_to_toggle_a_non_disableable_action():
+    """Silently doing nothing would leave a graph author with no signal."""
+    agent = _TogglableAgent()
+    with pytest.raises(InputValueError, match="cannot be toggled") as exc:
+        await run_node(
+            ToggleAgentAction(),
+            inputs={"agent": agent, "action_name": "locked", "enabled": False},
+        )
+    # names the agent, not its object repr — this message is user-facing
+    assert "toggle-node-test" in str(exc.value)
+    assert "object at 0x" not in str(exc.value)
+    assert agent.actions["locked"].enabled is True
+
+
+@pytest.mark.asyncio
+async def test_reports_the_effective_value_when_a_scene_override_takes_the_write(
+    tmp_path,
+):
+    """The write routes to the active override, leaving the global flag alone —
+    so the output reflects the override, not the agent's global setting."""
+    overrides = SceneAgentSettings(filepath=tmp_path / "x.json")
+    overrides.set_enabled("toggle-node-test", "toggleable", True)
+    scene = Scene()
+    scene.agent_overrides = overrides
+    agent = _TogglableAgent(scene=scene)
+
+    outputs = await run_node(
+        ToggleAgentAction(),
+        inputs={"agent": agent, "action_name": "toggleable", "enabled": False},
+    )
+
+    assert overrides.get_enabled("toggle-node-test", "toggleable") is False
+    # global flag untouched — the override took the write
+    assert agent.actions["toggleable"].enabled is True
+    assert outputs["enabled"] is False
+
+
+@pytest.mark.asyncio
+async def test_unknown_action_raises():
+    agent = _TogglableAgent()
+    with pytest.raises(InputValueError, match="Could not find action"):
+        await run_node(
+            ToggleAgentAction(),
+            inputs={"agent": agent, "action_name": "nope", "enabled": False},
+        )

tests/test_scene_agent_settings.py

@@ -63,6 +63,12 @@ class _ResolverAgent(Agent):
                     ),
                 },
             ),
+            "locked": AgentAction(
+                enabled=True,
+                label="Locked",
+                container=True,
+                can_be_disabled=False,
+            ),
         }
         self.scene = scene
         self.processing = 0
@@ -292,6 +298,50 @@ class TestAgentResolveEnabled:
         agent = _ResolverAgent(scene=_scene_with_overrides(overrides))
         assert agent.resolve_enabled("container") is True
 
+    def test_override_cannot_disable_a_non_disableable_action(self, tmp_path: Path):
+        """A hand-edited or legacy scene file must not turn off an action the
+        UI offers no Enable control for — it could never be undone."""
+        overrides = SceneAgentSettings(filepath=tmp_path / "x.json")
+        overrides.set_enabled("resolver-test", "locked", False)
+        agent = _ResolverAgent(scene=_scene_with_overrides(overrides))
+        assert agent.resolve_enabled("locked") is True
+
+
+class TestAgentWriteEnabled:
+    """`write_enabled` obeys the same declaration-wins rule as the resolver, so
+    a write can't persist a value `resolve_enabled` would then ignore."""
+
+    def test_writes_global_flag_when_disableable(self):
+        agent = _ResolverAgent(scene=None)
+        agent.write_enabled("container", False)
+        assert agent.actions["container"].enabled is False
+        assert agent.resolve_enabled("container") is False
+
+    def test_writes_active_override_when_disableable(self, tmp_path: Path):
+        overrides = SceneAgentSettings(filepath=tmp_path / "x.json")
+        overrides.set_enabled("resolver-test", "container", True)
+        agent = _ResolverAgent(scene=_scene_with_overrides(overrides))
+        agent.write_enabled("container", False)
+        assert overrides.get_enabled("resolver-test", "container") is False
+        # the global flag is left alone when an override took the write
+        assert agent.actions["container"].enabled is True
+
+    def test_refuses_to_disable_a_non_disableable_action(self):
+        agent = _ResolverAgent(scene=None)
+        agent.write_enabled("locked", False)
+        assert agent.actions["locked"].enabled is True
+        assert agent.resolve_enabled("locked") is True
+
+    def test_refuses_to_write_a_non_disableable_action_to_an_override(
+        self, tmp_path: Path
+    ):
+        overrides = SceneAgentSettings(filepath=tmp_path / "x.json")
+        overrides.set_enabled("resolver-test", "locked", True)
+        agent = _ResolverAgent(scene=_scene_with_overrides(overrides))
+        agent.write_enabled("locked", False)
+        assert overrides.get_enabled("resolver-test", "locked") is True
+        assert agent.resolve_enabled("locked") is True
+
 
 # ---------------------------------------------------------------------------
 # Complex-value overrides (flags / weights / lists / dicts)