fix(gateway): do not claim a destructive-slash opt-out that was not saved

Answering "Always Approve" on the /clear, /new, /reset and /undo
confirmation calls save_config_value("approvals.destructive_slash_confirm",
False) and then appended "Future /clear, /new, /reset, and /undo will run
without confirmation" unconditionally.

save_config_value catches its own exceptions and reports the outcome in the
return value, so the caller's try/except could never observe a failed write,
and the return value was ignored. On any install whose config.yaml is not
writable the user was told the preference stuck when it had not, and the
prompt returned on the next restart with no explanation.

Check the return value. The approved action still runs either way, but when
the write did not land, say so and point at the config key instead of
promising an opt-out that was never written.

gateway/run.py

@@ -19496,30 +19496,54 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
         async def _on_confirm(choice: str):
             if choice == "cancel":
                 return f"🟡 /{command} cancelled. Conversation unchanged."
+            persisted = False
             if choice == "always":
                 try:
                     from cli import save_config_value
-                    save_config_value("approvals.destructive_slash_confirm", False)
-                    logger.info(
-                        "User opted out of destructive slash confirm (session=%s)",
-                        session_key,
+                    # save_config_value swallows its own errors and reports the
+                    # outcome in the return value, so the try block alone says
+                    # nothing about whether the write landed.
+                    persisted = bool(
+                        save_config_value("approvals.destructive_slash_confirm", False)
                     )
+                    if persisted:
+                        logger.info(
+                            "User opted out of destructive slash confirm (session=%s)",
+                            session_key,
+                        )
+                    else:
+                        logger.warning(
+                            "Could not persist destructive_slash_confirm=false "
+                            "(session=%s); config.yaml is not writable",
+                            session_key,
+                        )
                 except Exception as exc:
                     logger.warning(
                         "Failed to persist destructive_slash_confirm=false: %s", exc,
                     )
             result = await execute()
             if choice == "always":
-                note = (
-                    "\n\nℹ️ Future /clear, /new, /reset, and /undo will run "
-                    "without confirmation. Re-enable via "
-                    "`approvals.destructive_slash_confirm: true` in config.yaml."
-                )
+                if persisted:
+                    note = (
+                        "\n\nℹ️ Future /clear, /new, /reset, and /undo will run "
+                        "without confirmation. Re-enable via "
+                        "`approvals.destructive_slash_confirm: true` in config.yaml."
+                    )
+                else:
+                    # The user did approve this run, so the action still goes
+                    # ahead, but the preference did not stick and the prompt
+                    # will be back next time. Say so rather than promising an
+                    # opt-out that was never written.
+                    note = (
+                        "\n\n⚠️ Could not save that preference (config.yaml is not "
+                        "writable), so /clear, /new, /reset, and /undo will ask "
+                        "again next time. To silence it permanently, set "
+                        "`approvals.destructive_slash_confirm: false` in config.yaml."
+                    )
                 if isinstance(result, str):
                     return result + note
-                # EphemeralReply or other — leave untouched; the opt-out note
-                # would otherwise mangle structured replies.  The persist itself
-                # already happened above; user gets the same UX next time.
+                # EphemeralReply or other: leave untouched, since the note would
+                # mangle structured replies.
                 return result
             return result
 

tests/gateway/test_destructive_slash_always_persist_report.py

@@ -0,0 +1,132 @@
+"""Answering "Always Approve" must not claim an opt-out that was not saved.
+
+``save_config_value`` swallows its own errors and reports the outcome through
+its return value, so the caller's ``try``/``except`` never sees a failed write.
+``_on_confirm`` ignored that return value and appended "Future /clear, /new,
+/reset, and /undo will run without confirmation" unconditionally, so on any
+install whose ``config.yaml`` is not writable (read-only bind mount, container
+recreation) the user was told the preference stuck when it had not. The prompt
+comes back on the next restart with no explanation.
+"""
+
+from __future__ import annotations
+
+import sys
+import types
+
+import pytest
+
+import gateway.run as gw
+
+
+class _Source:
+    platform = "telegram"
+
+
+class _Event:
+    source = _Source()
+
+
+def _runner():
+    """Bare object carrying only what _maybe_confirm_destructive_slash touches."""
+    obj = types.SimpleNamespace()
+    obj._read_user_config = lambda: {"approvals": {"destructive_slash_confirm": True}}
+    obj._session_key_for_source = lambda source: "sess-1"
+    obj._typed_command_prefix_for = lambda platform: "/"
+    captured = {}
+
+    async def _request_slash_confirm(*, event, command, title, message, handler):
+        captured["handler"] = handler
+        return "prompted"
+
+    obj._request_slash_confirm = _request_slash_confirm
+    obj._captured = captured
+    return obj
+
+
+async def _resolve(runner, choice, *, result="🧹 Conversation cleared."):
+    """Drive the gate, then invoke the captured handler with *choice*."""
+    async def _execute():
+        return result
+
+    await gw.GatewayRunner._maybe_confirm_destructive_slash(
+        runner,
+        event=_Event(),
+        command="clear",
+        title="Clear conversation",
+        detail="This discards history.",
+        execute=_execute,
+    )
+    return await runner._captured["handler"](choice)
+
+
+@pytest.fixture
+def fake_cli(monkeypatch):
+    """Install a stub `cli` module whose save_config_value outcome is settable."""
+    calls = []
+
+    module = types.ModuleType("cli")
+
+    def save_config_value(key_path, value):
+        calls.append((key_path, value))
+        return module._outcome
+
+    module.save_config_value = save_config_value
+    module._outcome = True
+    monkeypatch.setitem(sys.modules, "cli", module)
+    module.calls = calls
+    return module
+
+
+@pytest.mark.asyncio
+async def test_failed_persist_is_reported_as_failed(fake_cli):
+    fake_cli._outcome = False
+
+    out = await _resolve(_runner(), "always")
+
+    assert fake_cli.calls == [("approvals.destructive_slash_confirm", False)]
+    # The action the user approved still ran.
+    assert out.startswith("🧹 Conversation cleared.")
+    # ...but the message must not promise an opt-out that was never written.
+    assert "will run without confirmation" not in out
+    assert "Could not save that preference" in out
+
+
+@pytest.mark.asyncio
+async def test_successful_persist_still_reports_success(fake_cli):
+    fake_cli._outcome = True
+
+    out = await _resolve(_runner(), "always")
+
+    assert fake_cli.calls == [("approvals.destructive_slash_confirm", False)]
+    assert "will run without confirmation" in out
+    assert "Could not save that preference" not in out
+
+
+@pytest.mark.asyncio
+async def test_raising_persist_is_also_reported_as_failed(fake_cli):
+    def _boom(key_path, value):
+        raise OSError(30, "Read-only file system")
+
+    fake_cli.save_config_value = _boom
+
+    out = await _resolve(_runner(), "always")
+
+    assert out.startswith("🧹 Conversation cleared.")
+    assert "will run without confirmation" not in out
+
+
+@pytest.mark.asyncio
+async def test_once_neither_persists_nor_annotates(fake_cli):
+    out = await _resolve(_runner(), "once")
+
+    assert fake_cli.calls == []
+    assert out == "🧹 Conversation cleared."
+
+
+@pytest.mark.asyncio
+async def test_cancel_does_not_persist(fake_cli):
+    out = await _resolve(_runner(), "cancel")
+
+    assert fake_cli.calls == []
+    assert "cancelled" in out.lower()