Merge branch 'main' into local-branch-python-add-reset-to-workflow

python/packages/declarative/tests/test_declarative_approval_binding.py

@@ -0,0 +1,528 @@
+# Copyright (c) Microsoft. All rights reserved.
+# pyright: reportUnknownParameterType=false, reportUnknownArgumentType=false
+# pyright: reportMissingParameterType=false, reportUnknownMemberType=false
+# pyright: reportPrivateUsage=false, reportUnknownVariableType=false
+# pyright: reportGeneralTypeIssues=false
+
+"""Regression tests pinning the approval-flow binding contract.
+
+The resumed invocation MUST come from the framework-delivered
+``original_request`` payload (the data the reviewer approved) for both
+``InvokeFunctionTool`` and ``InvokeMcpTool``. These tests verify that:
+
+* Invocation parameters come from ``original_request``, not from any prior
+  side-channel state.
+* Concurrent pending approvals on the same executor do not swap.
+* Pre-existing state at old approval keys is ignored entirely.
+* Resume works on a freshly constructed executor (checkpoint-restore
+  simulation), without any prior ``ctx.state`` write.
+* For MCP, ``connection_name`` is sourced from the approval payload and
+  ``headers`` are re-evaluated from the action definition on resume.
+"""
+
+import sys
+from dataclasses import dataclass
+from typing import Any
+from unittest.mock import AsyncMock, MagicMock
+
+import pytest
+
+try:
+    import powerfx  # noqa: F401
+
+    _powerfx_available = True
+except (ImportError, RuntimeError):
+    _powerfx_available = False
+
+pytestmark = pytest.mark.skipif(
+    not _powerfx_available or sys.version_info >= (3, 14),
+    reason="PowerFx engine not available (requires dotnet runtime)",
+)
+
+from agent_framework import Content  # noqa: E402
+
+from agent_framework_declarative._workflows import (  # noqa: E402
+    DECLARATIVE_STATE_KEY,
+    ActionComplete,
+    InvokeFunctionToolExecutor,
+    MCPToolApprovalRequest,
+    MCPToolHandler,
+    MCPToolInvocation,
+    MCPToolResult,
+    ToolApprovalRequest,
+    ToolApprovalResponse,
+)
+from agent_framework_declarative._workflows._declarative_base import DeclarativeWorkflowState  # noqa: E402
+from agent_framework_declarative._workflows._executors_mcp import (  # noqa: E402
+    InvokeMcpToolActionExecutor,
+)
+
+# ---------------------------------------------------------------------------
+# Fixtures
+# ---------------------------------------------------------------------------
+
+
+@pytest.fixture
+def mock_state() -> MagicMock:
+    """In-memory mock of the underlying State."""
+    state = MagicMock()
+    state._data = {}
+
+    def _get(key: str, default: Any = None) -> Any:
+        return state._data.get(key, default)
+
+    def _set(key: str, value: Any) -> None:
+        state._data[key] = value
+
+    def _has(key: str) -> bool:
+        return key in state._data
+
+    def _delete(key: str) -> None:
+        state._data.pop(key, None)
+
+    state.get = MagicMock(side_effect=_get)
+    state.set = MagicMock(side_effect=_set)
+    state.has = MagicMock(side_effect=_has)
+    state.delete = MagicMock(side_effect=_delete)
+    return state
+
+
+@pytest.fixture
+def mock_context(mock_state: MagicMock) -> MagicMock:
+    ctx = MagicMock()
+    ctx.state = mock_state
+    ctx.send_message = AsyncMock()
+    ctx.yield_output = AsyncMock()
+    ctx.request_info = AsyncMock()
+    return ctx
+
+
+def _seed_state(mock_state: MagicMock) -> None:
+    mock_state._data[DECLARATIVE_STATE_KEY] = {
+        "Inputs": {},
+        "Outputs": {},
+        "Local": {},
+        "Custom": {},
+        "System": {
+            "ConversationId": "00000000-0000-0000-0000-000000000000",
+            "LastMessage": {"Text": "", "Id": ""},
+            "LastMessageText": "",
+            "LastMessageId": "",
+        },
+        "Agent": {},
+        "Conversation": {"messages": [], "history": []},
+    }
+
+
+class _RecordingMcpHandler(MCPToolHandler):
+    def __init__(self, result: MCPToolResult | None = None) -> None:
+        self.result = result or MCPToolResult(outputs=[Content.from_text("ok")])
+        self.invocations: list[MCPToolInvocation] = []
+
+    @property
+    def call_count(self) -> int:
+        return len(self.invocations)
+
+    @property
+    def last(self) -> MCPToolInvocation | None:
+        return self.invocations[-1] if self.invocations else None
+
+    async def invoke_tool(self, invocation: MCPToolInvocation) -> MCPToolResult:
+        self.invocations.append(invocation)
+        return self.result
+
+
+# ---------------------------------------------------------------------------
+# InvokeFunctionTool: approval-binding regression
+# ---------------------------------------------------------------------------
+
+
+class TestFunctionToolApprovalBinding:
+    def _action(self, *, fn_name: str = "my_tool") -> dict[str, Any]:
+        return {
+            "kind": "InvokeFunctionTool",
+            "id": "fn_action",
+            "functionName": fn_name,
+            "requireApproval": True,
+            "output": {"result": "Local.result"},
+        }
+
+    @pytest.mark.asyncio
+    async def test_request_id_matches_framework_pending_key(self, mock_state, mock_context) -> None:
+        """The id on the emitted ToolApprovalRequest must match the framework's pending-request key."""
+        from agent_framework_declarative._workflows._declarative_base import ActionTrigger
+
+        _seed_state(mock_state)
+
+        def my_tool(x: int) -> int:
+            return x
+
+        executor = InvokeFunctionToolExecutor(self._action(), tools={"my_tool": my_tool})
+        await executor.handle_action(ActionTrigger(), mock_context)
+
+        mock_context.request_info.assert_called_once()
+        emitted_request = mock_context.request_info.call_args[0][0]
+        framework_request_id = mock_context.request_info.call_args.kwargs["request_id"]
+        assert isinstance(emitted_request, ToolApprovalRequest)
+        assert emitted_request.request_id == framework_request_id
+
+    @pytest.mark.asyncio
+    async def test_resume_uses_request_payload_arguments(self, mock_state, mock_context) -> None:
+        _seed_state(mock_state)
+        call_log: list[int] = []
+
+        def my_tool(x: int) -> int:
+            call_log.append(x)
+            return x
+
+        executor = InvokeFunctionToolExecutor(self._action(), tools={"my_tool": my_tool})
+
+        request = ToolApprovalRequest(request_id="r-1", function_name="my_tool", arguments={"x": 1})
+        await executor.handle_approval_response(request, ToolApprovalResponse(approved=True), mock_context)
+
+        assert call_log == [1]
+
+    @pytest.mark.asyncio
+    async def test_concurrent_pending_approvals_do_not_swap(self, mock_state, mock_context) -> None:
+        """Two pending approvals, responses delivered out of order — each invocation uses its own payload."""
+        _seed_state(mock_state)
+        call_log: list[int] = []
+
+        def my_tool(x: int) -> int:
+            call_log.append(x)
+            return x
+
+        executor = InvokeFunctionToolExecutor(self._action(), tools={"my_tool": my_tool})
+
+        request_a = ToolApprovalRequest(request_id="r-A", function_name="my_tool", arguments={"x": 1})
+        request_b = ToolApprovalRequest(request_id="r-B", function_name="my_tool", arguments={"x": 999})
+
+        # Deliver response for B first, then for A. Each invocation must use its own payload.
+        await executor.handle_approval_response(request_b, ToolApprovalResponse(approved=True), mock_context)
+        await executor.handle_approval_response(request_a, ToolApprovalResponse(approved=True), mock_context)
+
+        assert call_log == [999, 1]
+
+    @pytest.mark.asyncio
+    async def test_resume_ignores_stale_state_at_old_approval_key(self, mock_state, mock_context) -> None:
+        """Pre-existing state at the OLD approval key is ignored — payload wins."""
+        _seed_state(mock_state)
+        call_log: list[int] = []
+
+        def my_tool(x: int) -> int:
+            call_log.append(x)
+            return x
+
+        executor = InvokeFunctionToolExecutor(self._action(), tools={"my_tool": my_tool})
+
+        # Poison the old key shape (no longer read by the executor).
+        mock_state._data["_tool_approval_state_fn_action"] = {"function_name": "other", "arguments": {"x": 999}}
+
+        request = ToolApprovalRequest(request_id="r-3", function_name="my_tool", arguments={"x": 7})
+        await executor.handle_approval_response(request, ToolApprovalResponse(approved=True), mock_context)
+
+        assert call_log == [7]
+        # The poison was never read or deleted by the executor.
+        assert "_tool_approval_state_fn_action" in mock_state._data
+
+    @pytest.mark.asyncio
+    async def test_fresh_executor_resume_works(self, mock_state, mock_context) -> None:
+        """Simulates checkpoint restore: a brand-new executor instance handles the approval response."""
+        _seed_state(mock_state)
+        call_log: list[int] = []
+
+        def my_tool(x: int) -> int:
+            call_log.append(x)
+            return x
+
+        # Pretend the executor that emitted the request is gone; a fresh one handles the response.
+        fresh = InvokeFunctionToolExecutor(self._action(), tools={"my_tool": my_tool})
+
+        request = ToolApprovalRequest(request_id="r-4", function_name="my_tool", arguments={"x": 42})
+        await fresh.handle_approval_response(request, ToolApprovalResponse(approved=True), mock_context)
+
+        assert call_log == [42]
+        mock_context.send_message.assert_called_once()
+        sent = mock_context.send_message.call_args[0][0]
+        assert isinstance(sent, ActionComplete)
+
+    @pytest.mark.asyncio
+    async def test_rejection_uses_request_payload_function_name(self, mock_state, mock_context) -> None:
+        _seed_state(mock_state)
+
+        def my_tool(x: int) -> int:
+            raise AssertionError("should not be called when rejected")
+
+        executor = InvokeFunctionToolExecutor(self._action(), tools={"my_tool": my_tool})
+
+        request = ToolApprovalRequest(request_id="r-5", function_name="my_tool", arguments={"x": 3})
+        await executor.handle_approval_response(
+            request, ToolApprovalResponse(approved=False, reason="not authorized"), mock_context
+        )
+
+        # The rejection message references the function name from the request payload.
+        local = mock_state._data[DECLARATIVE_STATE_KEY]["Local"]
+        assert local["result"]["rejected"] is True
+        assert local["result"]["reason"] == "not authorized"
+
+
+# ---------------------------------------------------------------------------
+# InvokeMcpTool: approval-binding regression
+# ---------------------------------------------------------------------------
+
+
+class TestMcpToolApprovalBinding:
+    def _action(self, *, headers: dict[str, Any] | None = None) -> dict[str, Any]:
+        action: dict[str, Any] = {
+            "kind": "InvokeMcpTool",
+            "id": "mcp_action",
+            "serverUrl": "https://mcp.example/api",
+            "toolName": "search",
+            "requireApproval": True,
+            "output": {"result": "Local.Result"},
+        }
+        if headers is not None:
+            action["headers"] = headers
+        return action
+
+    @pytest.mark.asyncio
+    async def test_request_id_matches_framework_pending_key(self, mock_state, mock_context) -> None:
+        """The id on the emitted MCPToolApprovalRequest must match the framework's pending-request key."""
+        from agent_framework_declarative._workflows._declarative_base import ActionTrigger
+
+        _seed_state(mock_state)
+        executor = InvokeMcpToolActionExecutor(self._action(), mcp_tool_handler=_RecordingMcpHandler())
+        await executor.handle_action(ActionTrigger(), mock_context)
+
+        mock_context.request_info.assert_called_once()
+        emitted_request = mock_context.request_info.call_args[0][0]
+        framework_request_id = mock_context.request_info.call_args.kwargs["request_id"]
+        assert isinstance(emitted_request, MCPToolApprovalRequest)
+        assert emitted_request.request_id == framework_request_id
+
+    @pytest.mark.asyncio
+    async def test_resume_uses_request_payload_fields(self, mock_state, mock_context) -> None:
+        _seed_state(mock_state)
+        handler = _RecordingMcpHandler()
+        executor = InvokeMcpToolActionExecutor(self._action(), mcp_tool_handler=handler)
+
+        request = MCPToolApprovalRequest(
+            request_id="r-1",
+            tool_name="search",
+            server_url="https://mcp.example/api",
+            server_label="prod",
+            arguments={"q": "x"},
+            connection_name="conn-A",
+        )
+        await executor.handle_approval_response(request, ToolApprovalResponse(approved=True), mock_context)
+
+        assert handler.call_count == 1
+        inv = handler.last
+        assert inv is not None
+        assert inv.tool_name == "search"
+        assert inv.server_url == "https://mcp.example/api"
+        assert inv.server_label == "prod"
+        assert inv.arguments == {"q": "x"}
+        assert inv.connection_name == "conn-A"
+
+    @pytest.mark.asyncio
+    async def test_concurrent_pending_mcp_approvals_do_not_swap(self, mock_state, mock_context) -> None:
+        _seed_state(mock_state)
+        handler = _RecordingMcpHandler()
+        executor = InvokeMcpToolActionExecutor(self._action(), mcp_tool_handler=handler)
+
+        request_a = MCPToolApprovalRequest(
+            request_id="r-A",
+            tool_name="search",
+            server_url="https://mcp.example/api",
+            server_label=None,
+            arguments={"q": "alpha"},
+            connection_name="conn-A",
+        )
+        request_b = MCPToolApprovalRequest(
+            request_id="r-B",
+            tool_name="search",
+            server_url="https://mcp.example/api",
+            server_label=None,
+            arguments={"q": "beta"},
+            connection_name="conn-B",
+        )
+
+        await executor.handle_approval_response(request_b, ToolApprovalResponse(approved=True), mock_context)
+        await executor.handle_approval_response(request_a, ToolApprovalResponse(approved=True), mock_context)
+
+        assert handler.call_count == 2
+        assert handler.invocations[0].arguments == {"q": "beta"}
+        assert handler.invocations[0].connection_name == "conn-B"
+        assert handler.invocations[1].arguments == {"q": "alpha"}
+        assert handler.invocations[1].connection_name == "conn-A"
+
+    @pytest.mark.asyncio
+    async def test_headers_reevaluated_from_action_def_on_resume(self, mock_state, mock_context) -> None:
+        """Headers come from the action definition (re-evaluated) so secrets are not in the payload."""
+        _seed_state(mock_state)
+        handler = _RecordingMcpHandler()
+        executor = InvokeMcpToolActionExecutor(
+            self._action(headers={"Authorization": "Bearer tk"}),
+            mcp_tool_handler=handler,
+        )
+
+        request = MCPToolApprovalRequest(
+            request_id="r-1",
+            tool_name="search",
+            server_url="https://mcp.example/api",
+            server_label=None,
+            arguments={"q": "x"},
+            connection_name=None,
+        )
+        await executor.handle_approval_response(request, ToolApprovalResponse(approved=True), mock_context)
+
+        assert handler.last is not None
+        assert handler.last.headers == {"Authorization": "Bearer tk"}
+
+    @pytest.mark.asyncio
+    async def test_mcp_resume_ignores_stale_state_at_old_approval_key(self, mock_state, mock_context) -> None:
+        _seed_state(mock_state)
+        handler = _RecordingMcpHandler()
+        executor = InvokeMcpToolActionExecutor(self._action(), mcp_tool_handler=handler)
+
+        mock_state._data["_mcp_tool_approval_state_mcp_action"] = {"poison": True}
+
+        request = MCPToolApprovalRequest(
+            request_id="r-1",
+            tool_name="search",
+            server_url="https://mcp.example/api",
+            server_label=None,
+            arguments={"q": "real"},
+            connection_name=None,
+        )
+        await executor.handle_approval_response(request, ToolApprovalResponse(approved=True), mock_context)
+
+        assert handler.call_count == 1
+        assert handler.last is not None
+        assert handler.last.arguments == {"q": "real"}
+        # The poison was never read or deleted by the executor.
+        assert "_mcp_tool_approval_state_mcp_action" in mock_state._data
+
+    @pytest.mark.asyncio
+    async def test_fresh_mcp_executor_resume_works(self, mock_state, mock_context) -> None:
+        """Checkpoint-restore simulation: fresh executor handles the response."""
+        _seed_state(mock_state)
+        handler = _RecordingMcpHandler()
+        fresh = InvokeMcpToolActionExecutor(self._action(), mcp_tool_handler=handler)
+
+        request = MCPToolApprovalRequest(
+            request_id="r-1",
+            tool_name="search",
+            server_url="https://mcp.example/api",
+            server_label=None,
+            arguments={"q": "fresh"},
+            connection_name=None,
+        )
+        await fresh.handle_approval_response(request, ToolApprovalResponse(approved=True), mock_context)
+
+        assert handler.call_count == 1
+        assert handler.last is not None
+        assert handler.last.arguments == {"q": "fresh"}
+
+    @pytest.mark.asyncio
+    async def test_request_payload_carries_connection_name(self, mock_state, mock_context) -> None:
+        """When emitting the approval request, connection_name flows into MCPToolApprovalRequest."""
+        from agent_framework_declarative._workflows._declarative_base import ActionTrigger
+
+        _seed_state(mock_state)
+        action = self._action()
+        action["connection"] = {"name": "conn-from-action"}
+        executor = InvokeMcpToolActionExecutor(action, mcp_tool_handler=_RecordingMcpHandler())
+
+        await executor.handle_action(ActionTrigger(), mock_context)
+
+        mock_context.request_info.assert_called_once()
+        request = mock_context.request_info.call_args[0][0]
+        assert isinstance(request, MCPToolApprovalRequest)
+        assert request.connection_name == "conn-from-action"
+
+    @pytest.mark.asyncio
+    async def test_request_payload_pins_conversation_id(self, mock_state, mock_context) -> None:
+        """Evaluated ``conversationId`` is pinned in ``metadata`` at request-emit time."""
+        from agent_framework_declarative._workflows._declarative_base import ActionTrigger
+
+        _seed_state(mock_state)
+        state = DeclarativeWorkflowState(mock_state)
+        state.set("Local.targetConversation", "conv-original")
+        action = self._action()
+        action["conversationId"] = "=Local.targetConversation"
+        executor = InvokeMcpToolActionExecutor(action, mcp_tool_handler=_RecordingMcpHandler())
+
+        await executor.handle_action(ActionTrigger(), mock_context)
+
+        mock_context.request_info.assert_called_once()
+        request = mock_context.request_info.call_args[0][0]
+        assert isinstance(request, MCPToolApprovalRequest)
+        assert request.metadata.get("conversation_id") == "conv-original"
+
+    @pytest.mark.asyncio
+    async def test_resume_routes_output_to_pinned_conversation_not_mutated_state(
+        self, mock_state, mock_context
+    ) -> None:
+        """Output appends to the conversation pinned on ``original_request``, not the
+        current state evaluation."""
+        _seed_state(mock_state)
+        state = DeclarativeWorkflowState(mock_state)
+        state.set("System.conversations.conv-original.messages", [])
+        state.set("System.conversations.conv-mutated.messages", [])
+        state.set("Local.targetConversation", "conv-mutated")
+
+        handler = _RecordingMcpHandler(MCPToolResult(outputs=[Content.from_text("approved-output")]))
+        action = self._action()
+        action["conversationId"] = "=Local.targetConversation"
+        executor = InvokeMcpToolActionExecutor(action, mcp_tool_handler=handler)
+
+        original_request = MCPToolApprovalRequest(
+            request_id="r-1",
+            tool_name="search",
+            server_url="https://mcp.example/api",
+            server_label=None,
+            arguments={"q": "x"},
+            connection_name=None,
+            metadata={"conversation_id": "conv-original"},
+        )
+        await executor.handle_approval_response(original_request, ToolApprovalResponse(approved=True), mock_context)
+
+        assert len(state.get("System.conversations.conv-original.messages") or []) == 1
+        assert state.get("System.conversations.conv-mutated.messages") == []
+
+    @pytest.mark.asyncio
+    async def test_resume_handles_legacy_request_without_new_fields(self, mock_state, mock_context) -> None:
+        """Resume tolerates payloads lacking ``connection_name`` / ``metadata`` (legacy pickle shape)."""
+
+        @dataclass
+        class _LegacyMCPApprovalRequest:
+            request_id: str
+            tool_name: str
+            server_url: str
+            server_label: str | None
+            arguments: dict[str, Any]
+            header_names: list[str]
+
+        _seed_state(mock_state)
+        handler = _RecordingMcpHandler()
+        executor = InvokeMcpToolActionExecutor(self._action(), mcp_tool_handler=handler)
+
+        legacy_request = _LegacyMCPApprovalRequest(
+            request_id="r-1",
+            tool_name="search",
+            server_url="https://mcp.example/api",
+            server_label=None,
+            arguments={"q": "x"},
+            header_names=[],
+        )
+        await executor.handle_approval_response(
+            legacy_request,  # type: ignore[arg-type]
+            ToolApprovalResponse(approved=True),
+            mock_context,
+        )
+
+        assert handler.call_count == 1
+        assert handler.last is not None
+        assert handler.last.connection_name is None

python/packages/declarative/tests/test_declarative_state_path_safety.py

@@ -0,0 +1,364 @@
+# Copyright (c) Microsoft. All rights reserved.
+# pyright: reportUnknownParameterType=false, reportUnknownArgumentType=false
+# pyright: reportMissingParameterType=false, reportUnknownMemberType=false
+# pyright: reportPrivateUsage=false, reportUnknownVariableType=false
+# pyright: reportGeneralTypeIssues=false
+
+"""Path-segment validation tests for DeclarativeWorkflowState.
+
+Path segments handed to ``get``/``set``/``append`` and ``{Variable.Path}``
+placeholders in ``interpolate_string`` are subject to three distinct rules
+that this module pins:
+
+- **Empty segments** (e.g. ``""``, ``"Local."``, ``"Local..foo"``) are rejected
+  by all of ``get``/``set``/``append`` and ``interpolate_string``. ``get`` and
+  ``interpolate_string`` return their default / leave the placeholder literal;
+  ``set`` and ``append`` raise ``ValueError``.
+- **Object-attribute segments** — segments that ``get`` would resolve via
+  ``getattr`` because the parent is a non-dict object — must match the safe
+  identifier shape ``[A-Za-z][A-Za-z0-9_]*``. Other shapes are rejected with a
+  warning log and the default is returned.
+- **Dict-keyed segments** — segments that resolve via dict lookup because the
+  parent is a ``dict`` — may use arbitrary non-empty string keys (e.g. UUIDs
+  or hyphenated identifiers like ``System.conversations.<uuid>.messages``).
+"""
+
+import logging
+from dataclasses import dataclass
+from typing import Any
+from unittest.mock import MagicMock
+
+import pytest
+
+from agent_framework_declarative._workflows import DeclarativeWorkflowState
+
+try:
+    import powerfx  # noqa: F401
+
+    _powerfx_available = True
+except (ImportError, RuntimeError):
+    _powerfx_available = False
+
+_requires_powerfx = pytest.mark.skipif(not _powerfx_available, reason="PowerFx engine not available")
+
+
+@pytest.fixture
+def mock_state() -> MagicMock:
+    """In-memory mock for the underlying State."""
+    ms = MagicMock()
+    ms._data = {}
+
+    def get(key: str, default: Any = None) -> Any:
+        return ms._data.get(key, default)
+
+    def set_(key: str, value: Any) -> None:
+        ms._data[key] = value
+
+    def has(key: str) -> bool:
+        return key in ms._data
+
+    def delete(key: str) -> None:
+        ms._data.pop(key, None)
+
+    ms.get = MagicMock(side_effect=get)
+    ms.set = MagicMock(side_effect=set_)
+    ms.has = MagicMock(side_effect=has)
+    ms.delete = MagicMock(side_effect=delete)
+    return ms
+
+
+@pytest.fixture
+def state(mock_state: MagicMock) -> DeclarativeWorkflowState:
+    s = DeclarativeWorkflowState(mock_state)
+    s.initialize()
+    return s
+
+
+@dataclass
+class _PlainObj:
+    """Non-dict object so ``get`` falls through to attribute access."""
+
+    text: str = "hi"
+
+
+# ---------------------------------------------------------------------------
+# get(): invalid paths return default
+# ---------------------------------------------------------------------------
+
+
+class TestGetRejectsInvalidPaths:
+    def test_rejects_dunder_segment_via_attribute_access(self, state: DeclarativeWorkflowState) -> None:
+        state.set("Local.obj", _PlainObj())
+        assert state.get("Local.obj.__class__") is None
+        assert state.get("Local.obj.__class__", default="DEF") == "DEF"
+
+    def test_rejects_full_env_exfil_chain(self, state: DeclarativeWorkflowState, monkeypatch) -> None:
+        sentinel = "agent-framework-path-safety-sentinel"
+        monkeypatch.setenv("AF_PATH_SAFETY_SENTINEL", sentinel)
+        state.set("Local.obj", _PlainObj())
+
+        result = state.get("Local.obj.__class__.__init__.__globals__.os.environ")
+
+        assert result is None
+        assert sentinel not in str(result)
+
+    def test_rejects_leading_underscore_via_attribute_access(self, state: DeclarativeWorkflowState) -> None:
+        state.set("Local.obj", _PlainObj())
+        assert state.get("Local.obj._private") is None
+
+    def test_rejects_invalid_chars_via_attribute_access(self, state: DeclarativeWorkflowState) -> None:
+        state.set("Local.obj", _PlainObj())
+        assert state.get("Local.obj.text bar") is None
+        assert state.get("Local.obj.text-bar") is None
+
+    def test_rejects_empty_path_and_empty_segments(self, state: DeclarativeWorkflowState) -> None:
+        assert state.get("") is None
+        assert state.get(".") is None
+        assert state.get("Local.") is None
+        assert state.get(".Local") is None
+
+    def test_warning_logged_on_rejected_attribute_segment(
+        self,
+        state: DeclarativeWorkflowState,
+        caplog: pytest.LogCaptureFixture,
+    ) -> None:
+        state.set("Local.obj", _PlainObj())
+        with caplog.at_level(logging.WARNING, logger="agent_framework_declarative._workflows._declarative_base"):
+            state.get("Local.obj.__class__")
+        assert any("rejecting attribute segment" in r.message for r in caplog.records)
+
+    def test_dict_keyed_dunder_is_not_attribute_access(self, state: DeclarativeWorkflowState) -> None:
+        """A literal dunder dict key is harmless because dict lookup never reaches getattr."""
+        state.set("Local.bag", {"__class__": "harmless-string"})
+        assert state.get("Local.bag.__class__") == "harmless-string"
+
+
+# ---------------------------------------------------------------------------
+# get(): legitimate paths continue to work
+# ---------------------------------------------------------------------------
+
+
+class TestGetAllowsValidPaths:
+    def test_underscore_inside_identifier(self, state: DeclarativeWorkflowState) -> None:
+        state.set("Local.user_input", "ok")
+        assert state.get("Local.user_input") == "ok"
+
+    def test_mixed_case_identifiers(self, state: DeclarativeWorkflowState) -> None:
+        state.set("Local.UserInput", "u1")
+        state.set("Local.userInput", "u2")
+        assert state.get("Local.UserInput") == "u1"
+        assert state.get("Local.userInput") == "u2"
+
+    def test_object_attribute_traversal_still_works(self, state: DeclarativeWorkflowState) -> None:
+        state.set("Local.msg", _PlainObj(text="hello"))
+        assert state.get("Local.msg.text") == "hello"
+
+    def test_nested_dict_traversal_still_works(self, state: DeclarativeWorkflowState) -> None:
+        state.set("Local.params", {"team": {"name": "alpha"}})
+        assert state.get("Local.params.team.name") == "alpha"
+
+    def test_uuid_and_hyphenated_dict_keys_are_allowed(self, state: DeclarativeWorkflowState) -> None:
+        """Conversation-id style paths use arbitrary dict keys (UUIDs / hyphens)."""
+        conv_id = "eb815014-06f1-4db6-b7c1-304ea135424f"
+        state.set(f"System.conversations.{conv_id}.messages", ["m1", "m2"])
+        assert state.get(f"System.conversations.{conv_id}.messages") == ["m1", "m2"]
+
+
+# ---------------------------------------------------------------------------
+# set() / append(): dict-keyed operations accept arbitrary string keys
+# ---------------------------------------------------------------------------
+
+
+class TestSetAndAppend:
+    def test_set_allows_underscore_inside_identifier(self, state: DeclarativeWorkflowState) -> None:
+        state.set("Local.user_input", "ok")
+        assert state.get("Local.user_input") == "ok"
+
+    def test_set_allows_uuid_and_hyphenated_dict_keys(self, state: DeclarativeWorkflowState) -> None:
+        conv_id = "conv-test-1"
+        state.set(f"System.conversations.{conv_id}.messages", [])
+        assert state.get(f"System.conversations.{conv_id}.messages") == []
+
+    def test_append_allows_uuid_and_hyphenated_dict_keys(self, state: DeclarativeWorkflowState) -> None:
+        conv_id = "conv-42"
+        state.append(f"System.conversations.{conv_id}.messages", {"role": "user", "text": "hi"})
+        msgs = state.get(f"System.conversations.{conv_id}.messages")
+        assert msgs == [{"role": "user", "text": "hi"}]
+
+    def test_workflow_inputs_still_read_only(self, state: DeclarativeWorkflowState) -> None:
+        with pytest.raises(ValueError, match="read-only"):
+            state.set("Workflow.Inputs.x", 1)
+
+
+# ---------------------------------------------------------------------------
+# set() / append(): malformed paths (empty segments) raise ValueError
+# ---------------------------------------------------------------------------
+
+
+class TestSetRejectsInvalidPaths:
+    @pytest.mark.parametrize("bad_path", ["", "Local.", "Local..foo", ".Local"])
+    def test_set_rejects_empty_segment(self, state: DeclarativeWorkflowState, bad_path: str) -> None:
+        with pytest.raises(ValueError, match="empty segments are not allowed"):
+            state.set(bad_path, "x")
+
+    @pytest.mark.parametrize("bad_path", ["", "Local.", "Local..foo", ".Local"])
+    def test_append_rejects_empty_segment(self, state: DeclarativeWorkflowState, bad_path: str) -> None:
+        with pytest.raises(ValueError, match="empty segments are not allowed"):
+            state.append(bad_path, "x")
+
+    def test_set_rejection_makes_no_partial_write(self, state: DeclarativeWorkflowState) -> None:
+        """Rejected set() must not create an unreachable entry in the state."""
+        state.set("Local.user_input", "pre")
+        with pytest.raises(ValueError):
+            state.set("Local.", "value")
+        local = state.get_state_data().get("Local", {})
+        assert "" not in local
+        assert local == {"user_input": "pre"}
+        assert state.get("Local.") is None
+        assert state.get("Local.user_input") == "pre"
+
+    def test_append_rejection_makes_no_partial_write(self, state: DeclarativeWorkflowState) -> None:
+        """Rejected append() must not create an unreachable entry in the state."""
+        state.set("Local.items", ["a"])
+        with pytest.raises(ValueError):
+            state.append("Local.", "value")
+        local = state.get_state_data().get("Local", {})
+        assert "" not in local
+        assert local == {"items": ["a"]}
+
+
+# ---------------------------------------------------------------------------
+# interpolate_string(): permissive matcher; get() enforces safety
+# ---------------------------------------------------------------------------
+
+
+class TestInterpolateString:
+    def test_ignores_dunder_payload(self, state: DeclarativeWorkflowState, monkeypatch) -> None:
+        sentinel = "agent-framework-interp-sentinel"
+        monkeypatch.setenv("AF_INTERP_SENTINEL", sentinel)
+        state.set("Local.obj", _PlainObj())
+
+        out = state.interpolate_string("X={Local.obj.__class__.__init__.__globals__.os.environ}")
+
+        assert sentinel not in out
+        assert out == "X="
+
+    def test_unknown_path_reduces_to_empty(self, state: DeclarativeWorkflowState) -> None:
+        assert state.interpolate_string("v={Local._private}") == "v="
+
+    @pytest.mark.parametrize(
+        "literal",
+        ["{foo-bar}", "{Ctrl+C}", "{not:a:path}", "{Local.}", "{}"],
+    )
+    def test_non_state_braced_tokens_left_literal(self, state: DeclarativeWorkflowState, literal: str) -> None:
+        assert state.interpolate_string(f"v={literal}") == f"v={literal}"
+
+    def test_allows_underscore_inside_identifier(self, state: DeclarativeWorkflowState) -> None:
+        state.set("Local.user_input", "hello")
+        assert state.interpolate_string("v={Local.user_input}") == "v=hello"
+
+    def test_resolves_nested_dict_path(self, state: DeclarativeWorkflowState) -> None:
+        state.set("Local.params", {"team": "alpha"})
+        assert state.interpolate_string("team={Local.params.team}") == "team=alpha"
+
+    @pytest.mark.parametrize(
+        ("key", "value"),
+        [
+            ("_id", "abc123"),
+            ("1", "one"),
+            ("2025", "year-bucket"),
+        ],
+    )
+    def test_resolves_dict_keyed_segments(self, state: DeclarativeWorkflowState, key: str, value: str) -> None:
+        state.set("Local.bag", {key: value})
+        assert state.interpolate_string(f"v={{Local.bag.{key}}}") == f"v={value}"
+
+    def test_resolves_uuid_conversation_key(self, state: DeclarativeWorkflowState) -> None:
+        conv_id = "eb815014-06f1-4db6-b7c1-304ea135424f"
+        state.set(f"System.conversations.{conv_id}.messages", ["hello"])
+        out = state.interpolate_string(f"m={{System.conversations.{conv_id}.messages}}")
+        assert out == "m=['hello']"
+
+    def test_end_to_end_send_activity_payload_neutralized(
+        self,
+        state: DeclarativeWorkflowState,
+        monkeypatch,
+    ) -> None:
+        sentinel = "agent-framework-e2e-sentinel"
+        monkeypatch.setenv("AF_E2E_SENTINEL", sentinel)
+        state.set("Local.toolResult", _PlainObj())
+
+        payload = "{Local.toolResult.__class__.__init__.__globals__.os.environ}"
+        evaluated = state.eval_if_expression(payload)
+        rendered = state.interpolate_string(evaluated) if isinstance(evaluated, str) else str(evaluated)
+
+        assert sentinel not in rendered
+        assert rendered == ""
+
+
+# ---------------------------------------------------------------------------
+# Regressions: PowerFx and internal temp-variable handling still work
+# ---------------------------------------------------------------------------
+
+
+@_requires_powerfx
+class TestPowerFxStillWorks:
+    def test_simple_powerfx_expression_evaluates(self, state: DeclarativeWorkflowState) -> None:
+        state.set("Local.x", 6)
+        state.set("Local.y", 7)
+        assert state.eval("=Local.x * Local.y") == 42
+
+    def test_internal_temp_message_text_still_works(self, state: DeclarativeWorkflowState) -> None:
+        """Long MessageText() results round-trip and the temp key is removed after eval."""
+        long_text = "A" * 600
+        state.set(
+            "Local.Messages",
+            [{"text": long_text, "contents": [{"type": "text", "text": long_text}]}],
+        )
+
+        result = state.eval("=Upper(MessageText(Local.Messages))")
+        assert result == "A" * 600
+
+        local = state.get_state_data().get("Local", {})
+        remaining = sorted(k for k in local if k.startswith("_TempMessageText"))
+        assert not remaining, f"Temporary keys remain in Local: {remaining}"
+
+    def test_message_text_eval_preserves_user_temp_value(self, state: DeclarativeWorkflowState) -> None:
+        """User state at the temp key path survives a long MessageText eval."""
+        long_text = "A" * 600
+        state.set("Local._TempMessageText0", "user-important-value")
+        state.set(
+            "Local.Messages",
+            [{"text": long_text, "contents": [{"type": "text", "text": long_text}]}],
+        )
+
+        result = state.eval("=Upper(MessageText(Local.Messages))")
+        assert result == "A" * 600
+        assert state.get("Local._TempMessageText0") == "user-important-value"
+
+    def test_message_text_eval_cleans_up_on_powerfx_failure(
+        self,
+        state: DeclarativeWorkflowState,
+        monkeypatch,
+    ) -> None:
+        """Temp key is removed even when PowerFx evaluation raises."""
+        from agent_framework_declarative._workflows import _declarative_base as base
+
+        class _FailingEngine:
+            def eval(self, *args: Any, **kwargs: Any) -> Any:
+                raise RuntimeError("boom")
+
+        monkeypatch.setattr(base, "Engine", _FailingEngine)
+
+        long_text = "A" * 600
+        state.set(
+            "Local.Messages",
+            [{"text": long_text, "contents": [{"type": "text", "text": long_text}]}],
+        )
+
+        with pytest.raises(RuntimeError, match="boom"):
+            state.eval("=Upper(MessageText(Local.Messages))")
+
+        local = state.get_state_data().get("Local", {})
+        remaining = sorted(k for k in local if k.startswith("_TempMessageText"))
+        assert not remaining, f"Temporary keys remain in Local after PowerFx failure: {remaining}"

python/packages/declarative/tests/test_function_tool_executor.py

@@ -35,14 +35,12 @@ pytestmark = pytest.mark.skipif(
 from agent_framework_declarative._workflows import (  # noqa: E402
     DECLARATIVE_STATE_KEY,
     FUNCTION_TOOL_REGISTRY_KEY,
-    TOOL_APPROVAL_STATE_KEY,
     ActionComplete,
     ActionTrigger,
     DeclarativeWorkflowBuilder,
     InvokeFunctionToolExecutor,
     ToolApprovalRequest,
     ToolApprovalResponse,
-    ToolApprovalState,
     ToolInvocationResult,
     WorkflowFactory,
 )
@@ -393,21 +391,6 @@ class TestToolApprovalTypes:
         assert response.approved is False
         assert response.reason == "Not authorized"
 
-    def test_approval_state(self):
-        """Test creating approval state for yield/resume."""
-        state = ToolApprovalState(
-            function_name="delete_user",
-            arguments={"user_id": "123"},
-            output_messages_var="Local.messages",
-            output_result_var="Local.result",
-            auto_send=True,
-        )
-        assert state.function_name == "delete_user"
-        assert state.arguments == {"user_id": "123"}
-        assert state.output_messages_var == "Local.messages"
-        assert state.output_result_var == "Local.result"
-        assert state.auto_send is True
-
 
 class TestInvokeFunctionToolEdgeCases:
     """Tests for edge cases and error handling."""
@@ -1075,13 +1058,6 @@ class TestApprovalFlow:
         # Should NOT have sent ActionComplete (workflow yields)
         mock_context.send_message.assert_not_called()
 
-        # Approval state should be saved in state
-        approval_key = f"{TOOL_APPROVAL_STATE_KEY}_approval_test"
-        saved_state = mock_state._data[approval_key]
-        assert isinstance(saved_state, ToolApprovalState)
-        assert saved_state.function_name == "my_tool"
-        assert saved_state.arguments == {"x": 5}
-
     @pytest.mark.asyncio
     async def test_approval_response_approved(self, mock_state, mock_context):
         """When approval response is approved, the tool should be invoked."""
@@ -1104,17 +1080,7 @@ class TestApprovalFlow:
 
         executor = InvokeFunctionToolExecutor(action_def, tools={"my_tool": my_tool})
 
-        # Pre-populate approval state (simulating what handle_action stores)
-        approval_key = f"{TOOL_APPROVAL_STATE_KEY}_approval_approved"
-        mock_state._data[approval_key] = ToolApprovalState(
-            function_name="my_tool",
-            arguments={"x": 7},
-            output_messages_var=None,
-            output_result_var="Local.result",
-            auto_send=True,
-        )
-
-        # Simulate the response
+        # Simulate the response — invocation params come from original_request
         original_request = ToolApprovalRequest(
             request_id="req-123",
             function_name="my_tool",
@@ -1124,7 +1090,7 @@ class TestApprovalFlow:
 
         await executor.handle_approval_response(original_request, response, mock_context)
 
-        # Tool should have been called
+        # Tool should have been called with the approved arguments
         assert call_log == [7]
 
         # ActionComplete should have been sent
@@ -1132,9 +1098,6 @@ class TestApprovalFlow:
         sent = mock_context.send_message.call_args[0][0]
         assert isinstance(sent, ActionComplete)
 
-        # Approval state should be cleaned up
-        assert approval_key not in mock_state._data
-
     @pytest.mark.asyncio
     async def test_approval_response_rejected(self, mock_state, mock_context):
         """When approval response is rejected, rejection status should be stored."""
@@ -1154,16 +1117,6 @@ class TestApprovalFlow:
 
         executor = InvokeFunctionToolExecutor(action_def, tools={"my_tool": my_tool})
 
-        # Pre-populate approval state
-        approval_key = f"{TOOL_APPROVAL_STATE_KEY}_approval_rejected"
-        mock_state._data[approval_key] = ToolApprovalState(
-            function_name="my_tool",
-            arguments={"x": 5},
-            output_messages_var=None,
-            output_result_var="Local.result",
-            auto_send=True,
-        )
-
         original_request = ToolApprovalRequest(
             request_id="req-456",
             function_name="my_tool",
@@ -1185,36 +1138,6 @@ class TestApprovalFlow:
         assert result["reason"] == "Not authorized"
         assert result["approved"] is False
 
-    @pytest.mark.asyncio
-    async def test_approval_response_missing_state(self, mock_state, mock_context):
-        """When approval state is missing on resume, should log error and complete."""
-        self._init_state(mock_state)
-
-        action_def = {
-            "kind": "InvokeFunctionTool",
-            "id": "missing_state_test",
-            "functionName": "my_tool",
-            "requireApproval": True,
-            "output": {"result": "Local.result"},
-        }
-
-        executor = InvokeFunctionToolExecutor(action_def, tools={})
-
-        # Don't populate approval state - simulate missing state
-        original_request = ToolApprovalRequest(
-            request_id="req-789",
-            function_name="my_tool",
-            arguments={},
-        )
-        response = ToolApprovalResponse(approved=True)
-
-        await executor.handle_approval_response(original_request, response, mock_context)
-
-        # Should still send ActionComplete
-        mock_context.send_message.assert_called_once()
-        sent = mock_context.send_message.call_args[0][0]
-        assert isinstance(sent, ActionComplete)
-
 
 # ============================================================================
 # State registry tool lookup (lines 255-257)

python/packages/declarative/tests/test_graph_coverage.py

@@ -2765,7 +2765,7 @@ class TestLongMessageTextHandling:
         assert temp_var is None
 
     async def test_long_message_text_stored_in_temp_variable(self, mock_state):
-        """Test that long MessageText results are stored in temp variables."""
+        """Long MessageText results round-trip and the temp key is removed after eval."""
         state = DeclarativeWorkflowState(mock_state)
         state.initialize()
 
@@ -2777,9 +2777,9 @@ class TestLongMessageTextHandling:
         result = state.eval("=Upper(MessageText(Local.Messages))")
         assert result == "A" * 600  # Upper on 'A' is still 'A'
 
-        # A temp variable should have been created
-        temp_var = state.get("Local._TempMessageText0")
-        assert temp_var == long_text
+        local = state.get_state_data().get("Local", {})
+        remaining = sorted(k for k in local if k.startswith("_TempMessageText"))
+        assert not remaining, f"Temporary keys remain in Local: {remaining}"
 
     async def test_find_with_long_message_text(self, mock_state):
         """Test Find function works with long MessageText stored in temp variable."""

python/packages/declarative/tests/test_invoke_mcp_tool_executor.py

@@ -403,7 +403,6 @@ class TestApprovalFlow:
     async def test_approval_required_emits_request_and_yields(self, mock_state, mock_context) -> None:  # type: ignore[no-untyped-def]
         from agent_framework_declarative._workflows._declarative_base import ActionTrigger
         from agent_framework_declarative._workflows._executors_mcp import (
-            _MCP_APPROVAL_STATE_KEY,
             InvokeMcpToolActionExecutor,
             MCPToolApprovalRequest,
         )
@@ -439,18 +438,12 @@ class TestApprovalFlow:
         # Handler not invoked yet.
         assert handler.call_count == 0
 
-        # Approval state stored.
-        approval_key = f"{_MCP_APPROVAL_STATE_KEY}_mcp_action"
-        assert approval_key in mock_state._data
-
     @pytest.mark.asyncio
     async def test_approval_response_approved_invokes_handler(self, mock_state, mock_context) -> None:  # type: ignore[no-untyped-def]
         from agent_framework_declarative._workflows import ActionComplete, ToolApprovalResponse
         from agent_framework_declarative._workflows._executors_mcp import (
-            _MCP_APPROVAL_STATE_KEY,
             InvokeMcpToolActionExecutor,
             MCPToolApprovalRequest,
-            _MCPToolApprovalState,
         )
 
         _seed_state(mock_state)
@@ -458,24 +451,11 @@ class TestApprovalFlow:
         executor = InvokeMcpToolActionExecutor(
             _action(
                 require_approval=True,
+                headers={"Authorization": "Bearer tk"},
                 output={"result": "Local.Result"},
             ),
             mcp_tool_handler=handler,
         )
-        # Pre-populate approval state.
-        approval_key = f"{_MCP_APPROVAL_STATE_KEY}_mcp_action"
-        mock_state._data[approval_key] = _MCPToolApprovalState(
-            server_url="https://mcp.example/api",
-            tool_name="search",
-            server_label=None,
-            arguments={"q": "x"},
-            connection_name=None,
-            headers_def={"Authorization": "Bearer tk"},
-            auto_send=False,
-            conversation_id_expr=None,
-            output_messages_path=None,
-            output_result_path="Local.Result",
-        )
         await executor.handle_approval_response(
             MCPToolApprovalRequest(
                 request_id="req-1",
@@ -491,10 +471,12 @@ class TestApprovalFlow:
         assert handler.call_count == 1
         inv = handler.last_invocation
         assert inv is not None
-        # Headers are re-evaluated from headers_def.
+        # Invocation fields source from the approval request payload.
+        assert inv.tool_name == "search"
+        assert inv.server_url == "https://mcp.example/api"
+        assert inv.arguments == {"q": "x"}
+        # Headers are re-evaluated from the action definition on resume.
         assert inv.headers == {"Authorization": "Bearer tk"}
-        # Approval state was cleaned up.
-        assert approval_key not in mock_state._data
         # ActionComplete was sent.
         mock_context.send_message.assert_called_once()
         sent = mock_context.send_message.call_args[0][0]
@@ -504,10 +486,8 @@ class TestApprovalFlow:
     async def test_approval_response_rejected_assigns_error(self, mock_state, mock_context) -> None:  # type: ignore[no-untyped-def]
         from agent_framework_declarative._workflows import ToolApprovalResponse
         from agent_framework_declarative._workflows._executors_mcp import (
-            _MCP_APPROVAL_STATE_KEY,
             InvokeMcpToolActionExecutor,
             MCPToolApprovalRequest,
-            _MCPToolApprovalState,
         )
 
         _seed_state(mock_state)
@@ -519,19 +499,6 @@ class TestApprovalFlow:
             ),
             mcp_tool_handler=handler,
         )
-        approval_key = f"{_MCP_APPROVAL_STATE_KEY}_mcp_action"
-        mock_state._data[approval_key] = _MCPToolApprovalState(
-            server_url="https://mcp.example/api",
-            tool_name="search",
-            server_label=None,
-            arguments={},
-            connection_name=None,
-            headers_def=None,
-            auto_send=True,
-            conversation_id_expr=None,
-            output_messages_path=None,
-            output_result_path="Local.Result",
-        )
         await executor.handle_approval_response(
             MCPToolApprovalRequest(
                 request_id="req-2",