mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
Fixes microsoft/agent-framework#3295. When the OpenAI Responses chat client sends a request that carries previous_response_id / conversation_id / conversation, the server already has the prior turn's response items and rejects duplicates with "Duplicate item found with id fc_xxx". The chat client was re-sending them inline whenever the input messages still carried the items in additional_properties (workflow replay, history providers, etc.), which broke any tool-using agent with persistent history. Decisions: - Single chokepoint: _prepare_message_for_openai. When the resulting request uses service-side storage, drop function_call, reasoning, approval-request/response, and local-shell-call items from the wire input. Keep function_result with its call_id; the server pairs it to the prior function_call via that key. - function_result is preserved unconditionally except for the local-shell variant, which carries its own server-issued item id. - No public API change. Wire format change is subtractive and only on requests that would otherwise 400. - Re-pointed the strict-xfail in test_full_conversation.py from #4047 to #3295. Kept xfail because the test asserts executor-level session-id clearing, which is the defense-in-depth half tracked by 3295-03; this slice closes the wire-level half. Files: - python/packages/openai/agent_framework_openai/_chat_client.py: strip rule applied alongside the existing reasoning-item branch. - python/packages/openai/tests/openai/test_openai_chat_client.py: four new tests pin the contract (function_call, approval, local-shell-call stripped under storage; everything kept without storage). Updated pre-existing tests that exercised the storage-on path to either pass request_uses_service_side_storage=False explicitly or assert the new strip behavior. - python/packages/foundry/tests/foundry/test_foundry_chat_client.py: same explicit storage-off opt-in for the inherited test. - python/packages/core/tests/workflow/test_full_conversation.py: re-pointed xfail reason to #3295 and the executor-level follow-up. Notes for next iteration: - 3295-01 (HITL wire-format validation against live OpenAI/Foundry) was not run; it requires the user's API credentials. The PRD design is locked but the empirical confirmation is still pending. If script 3 fails on either provider, this slice may need to be revisited. - 3295-03 (clear service_session_id in AgentExecutor on full-history replay) remains open. After it lands the xfail in test_full_conversation.py can be removed. - pytest was not run in this iteration because uv-based pytest commands required interactive approval. Validation rests on careful reading; next iteration should run the openai + core test suites.
This commit is contained in:
committed by
GitHub
Unverified
parent
ab09246dc4
commit
09a3d0d307
@@ -1409,29 +1409,31 @@ class RawOpenAIChatClient( # type: ignore[misc]
|
||||
}
|
||||
additional_properties = message.additional_properties
|
||||
replays_local_storage = "_attribution" in additional_properties
|
||||
uses_service_side_storage = request_uses_service_side_storage and not replays_local_storage
|
||||
# Reasoning items are only valid in input when they directly preceded a function_call
|
||||
# in the same response. Including a reasoning item that preceded a text response
|
||||
# (i.e. no function_call in the same message) causes an API error:
|
||||
# "reasoning was provided without its required following item."
|
||||
#
|
||||
# Local storage is stricter: response-scoped reasoning items (rs_*) cannot be replayed
|
||||
# back to the service unless that message is using service-side storage.
|
||||
# In that mode we omit reasoning items and rely on function call + tool output replay.
|
||||
has_function_call = any(c.type == "function_call" for c in message.contents)
|
||||
# Server-issued response item identities (function_call fc_*, reasoning rs_*, approval IDs,
|
||||
# local-shell-call IDs) must not be re-sent inline when the request carries
|
||||
# previous_response_id / conversation_id / conversation: the server already has them via
|
||||
# the prior response and rejects duplicates with "Duplicate item found with id ...".
|
||||
# function_result keeps its call_id and the server pairs it to the prior function_call via
|
||||
# that key. See microsoft/agent-framework#3295. The strip is gated on the request-level
|
||||
# flag, not a message-level one: HistoryProvider-attributed messages
|
||||
# (replays_local_storage) still need stripping when the request also carries a continuation
|
||||
# marker, since the server-stored items would otherwise duplicate the inline ones. Without
|
||||
# storage, standalone reasoning items are invalid per the API ("reasoning was provided
|
||||
# without its required following item"), so the reasoning branch always drops.
|
||||
for content in message.contents:
|
||||
match content.type:
|
||||
case "text_reasoning":
|
||||
if not uses_service_side_storage or not has_function_call:
|
||||
continue # reasoning not followed by a function_call is invalid in input
|
||||
reasoning = self._prepare_content_for_openai(
|
||||
message.role,
|
||||
content,
|
||||
replays_local_storage=replays_local_storage,
|
||||
)
|
||||
if reasoning:
|
||||
all_messages.append(reasoning)
|
||||
continue
|
||||
case "function_result":
|
||||
if request_uses_service_side_storage:
|
||||
props = content.additional_properties or {}
|
||||
# Local-shell variant serializes as `local_shell_call` carrying a server-issued id;
|
||||
# plain function_call_output pairs by call_id and is safe under storage.
|
||||
if (
|
||||
props.get(OPENAI_SHELL_OUTPUT_TYPE_KEY) == OPENAI_SHELL_OUTPUT_TYPE_LOCAL_SHELL_CALL
|
||||
and props.get(OPENAI_LOCAL_SHELL_CALL_ITEM_ID_KEY)
|
||||
):
|
||||
continue
|
||||
new_args: dict[str, Any] = {}
|
||||
new_args.update(
|
||||
self._prepare_content_for_openai(
|
||||
@@ -1443,6 +1445,8 @@ class RawOpenAIChatClient( # type: ignore[misc]
|
||||
if new_args:
|
||||
all_messages.append(new_args)
|
||||
case "function_call":
|
||||
if request_uses_service_side_storage:
|
||||
continue
|
||||
function_call = self._prepare_content_for_openai(
|
||||
message.role,
|
||||
content,
|
||||
@@ -1451,6 +1455,8 @@ class RawOpenAIChatClient( # type: ignore[misc]
|
||||
if function_call:
|
||||
all_messages.append(function_call)
|
||||
case "function_approval_response" | "function_approval_request":
|
||||
if request_uses_service_side_storage:
|
||||
continue
|
||||
prepared = self._prepare_content_for_openai(
|
||||
message.role,
|
||||
content,
|
||||
@@ -1463,6 +1469,12 @@ class RawOpenAIChatClient( # type: ignore[misc]
|
||||
# top-level mcp_call input item; the result side emits an
|
||||
# internal marker that `_prepare_messages_for_openai`
|
||||
# coalesces onto the matching call (or drops if unmatched).
|
||||
# The mcp_call item carries the model-emitted call_id as its
|
||||
# server-side `id`, so under continuation it would duplicate
|
||||
# the prior response's items (#3295). Drop the call here; the
|
||||
# orphan result is dropped by the coalesce step that follows.
|
||||
if request_uses_service_side_storage:
|
||||
continue
|
||||
prepared_mcp = self._prepare_content_for_openai(
|
||||
message.role,
|
||||
content,
|
||||
|
||||
Reference in New Issue
Block a user