Python: Hosted declarative#5530
Conversation
…rt executor The declarative start executor (JoinExecutor) only advertised dict and str in its input_types, so WorkflowAgent.__init__ rejected it with 'Workflow's start executor cannot handle list[Message]'. Add list[Message] to the JoinExecutor handler annotation and add a matching branch in DeclarativeActionExecutor._ensure_state_initialized that extracts the last user-message text and falls through to the string-input initialization path, so =System.LastMessageText works end-to-end via as_agent(). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When Workflow.as_agent() is invoked with a list[Message], the start executor now populates Conversation.messages / Conversation.history / System.conversations.{id}.messages with prior turns only (excluding the latest user message), and surfaces the latest user message via Inputs.input and System.LastMessage*. This matches InvokeAzureAgent's contract that the messages binding holds prior turns and the executor itself appends the new user input before invoking, avoiding double-append of the trailing user turn while preserving full history (incl. assistant/system/tool roles and multi-modal content) for downstream actions.
MessageRole and other str-subclass Enums passed isinstance(v, str) and were forwarded to pythonnet unchanged. pythonnet then raised 'MessageRole value cannot be converted to System.String' for every PowerFx primitive when ConditionGroup/Expr eval walked the symbol table containing Conversation.messages. Reduce Enum members to their underlying value before the primitive check so eval sees plain strings/ints.
_handle_inner_workflow only forwarded the latest user turn to WorkflowAgent.run, even though _handle_inner_agent already prepends history fetched from Foundry storage to the messages it sends a regular agent. Declarative workflows reset Conversation.messages on every run (state.initialize), so checkpoint replay alone does not give them prior turns - the host has to pass them in, the same way it does for non-workflow agents. Mirror that contract: fetch context.get_history() and pass [*history, *input_messages] to the workflow agent.
There was a problem hiding this comment.
Automated Code Review
Reviewers: 4 | Confidence: 89%
✓ Correctness
The PR correctly adds list[Message] support to declarative workflow initialization (for WorkflowAgent/as_agent()), adds Enum coercion to _make_powerfx_safe, and wires up conversation history in the Foundry hosting workflow handler. The logic is sound: the message-list splitting, history population, and System.LastMessage* mirroring all match the described .NET DefaultTransform semantics. One minor operator-precedence issue exists that is functionally harmless but could confuse future maintainers.
✓ Security Reliability
This PR adds support for list[Message] inputs in declarative workflows (from WorkflowAgent/as_agent()) and fetches prior conversation history in the Foundry hosting layer. The changes follow established patterns already present in the codebase (e.g., _handle_inner_agent already does the same history fetch + merge). Edge cases (empty message list, no user messages, Enum recursion in _make_powerfx_safe) are handled correctly. The conversation_id used in state paths originates from framework-controlled state, not user input, so there is no path-injection risk. No security or reliability issues found.
✓ Test Coverage
The PR adds ~70 lines of complex branching logic in
_ensure_state_initializedto handlelist[Message]triggers, Enum coercion in_make_powerfx_safe, and history-prepending in_handle_inner_workflow. Only one integration-level test is added (test_as_agent_round_trip_with_last_message_text), which exercises the simplest happy path (single user string → WorkflowAgent → list[Message] with one user message). Several important code paths and edge cases lack test coverage: the multi-turn history splitting logic, the no-user-message fallback branch, Conversation.messages/history population, and the Enum coercion change. The_handle_inner_workflowchange has no test at all.
✓ Design Approach
The change fixes the string-based
System.LastMessageTextregression, but the broader design overcorrects in two ways: it narows the new agent-facinglist[Message]path back down to text for the current user turn, so multimodal/tool-bearing inputs still cannot round-trip through declarative workflows, and it feeds full conversation history intoWorkflowAgent.run()even on checkpoint-resume paths where the agent explicitly requires only pending-request responses. Both issues point to contract mismatches rather than isolated implementation bugs.
Automated review by alliscode's agents
|
|
||
| # Locate the trailing user message: WorkflowAgent merges session | ||
| # history with the caller's new input and forwards the combined | ||
| # list, so the most recent user message represents "this turn" |
There was a problem hiding this comment.
This reduces the newest user turn to last_user_msg.text, but Message.text only concatenates text content. Downstream, InvokeAzureAgentExecutor reconstructs the turn as Message(role="user", contents=[input_text]), so images, tool responses, approvals, or any other non-text content on the current turn are silently dropped. A safer design is to keep the full Message in state and only derive System.LastMessageText as a compatibility view.
| @@ -307,7 +320,7 @@ async def _handle_inner_workflow( | |||
|
|
|||
There was a problem hiding this comment.
Passing full_messages here breaks the restored-checkpoint path. _handle_inner_workflow() restores the latest checkpoint just above, and WorkflowAgent uses that to rebuild pending_requests. On resume, _process_pending_requests() calls _extract_function_responses(), which explicitly rejects non-response history content while requests are pending. A resumed approval/tool-call flow will now receive the entire transcript and fail by design. The host should pass only the new response items when resuming pending requests, and reserve full_messages for fresh turns.
|
|
||
| assert "Hello there" in response.text, ( | ||
| f"Expected 'Hello there' in agent response text but got: {response.text!r}" | ||
| ) |
There was a problem hiding this comment.
This test only exercises the simplest case: a single user string through as_agent(). The new _ensure_state_initialized branch has ~70 lines of branching logic for multi-message history splitting, the no-user-message fallback, and Conversation.messages/history population — none of which are covered. Consider adding unit-level tests that call handle_action with multi-message list[Message] inputs and inspect state, e.g.: (1) a three-message sequence [user, assistant, user] verifying System.LastMessageText equals the last user text and Conversation.messages contains only the first two; (2) an assistant-only list verifying the no-user-message fallback populates System.LastMessageText from the tail.
This pull request enhances the handling of conversation history and message inputs in declarative workflow agents, ensuring full compatibility with agent-based workflows and improved cross-turn context. The changes ensure that when workflows are invoked as agents, they receive the complete message history (not just the latest user turn), and that key fields like
System.LastMessageTextare correctly populated for backward compatibility. The update also includes a regression test to verify this behavior.