Skip to content

fix(openai): reserialize response body when MCP approval is required (#839) - #855

Open
Artemon-line wants to merge 2 commits into
mainfrom
839-mcp-approval-requests-are-omitted-from-the-serialized-client-response
Open

fix(openai): reserialize response body when MCP approval is required (#839)#855
Artemon-line wants to merge 2 commits into
mainfrom
839-mcp-approval-requests-are-omitted-from-the-serialized-client-response

Conversation

@Artemon-line

Copy link
Copy Markdown

Summary

When an MCP tool call requires approval, McpDispatchFilter::on_response_body detects the approval requirement, constructs an mcp_approval_request item, and appends it to state.accumulated_output. However, because AgenticLoopFilter had already formatted the HTTP response body before McpDispatchFilter ran, and McpDispatchFilter had read-only response body access, the mcp_approval_request event was missing from the serialized HTTP client response.

This change:

  1. Gives McpDispatchFilter read-write response body access (BodyAccess::ReadWrite).
  2. Exposes finalize_response_body on ResponsesState to re-serialize the response body from accumulated_output and usage.
  3. Calls state.finalize_response_body(body) inside handle_approval_required when appending an approval request event.
  4. Updates AgenticLoopFilter calls to use state.finalize_response_body(body).
  5. Adds a unit test verifying that mcp_approval_request is correctly serialized into the returned HTTP response body bytes.

Related issue

Closes #839

Validation

  • Unit tests: cargo test -p praxis-ai-apis -- mcp_dispatch::tests
  • Workspace tests: make test
  • make lint and make fmt

Checklist

  • I reviewed every changed line and can explain the change.
  • New capabilities include an example config and functional example test. (N/A — bug fix)
  • User-facing behavior and generated documentation are updated. (N/A)
  • Performance-sensitive changes include appropriate benchmark or load-test evidence. (N/A)
  • Commits are signed and include a Signed-off-by trailer.

Breaking changes

None.

@Artemon-line
Artemon-line requested review from a team and nerdalert September 1, 2026 13:49
@Artemon-line Artemon-line changed the title ix(openai): reserialize response body when MCP approval is required (#839) fix(openai): reserialize response body when MCP approval is required (#839) Sep 1, 2026
Signed-off-by: Artemy <ahladenk@redhat.com>
@leseb

leseb commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

@Artemon-line next time use your fork. Thanks

/// Preserves the full object from the request so filters can
/// inspect both the strategy type and any parameters.
pub context_management: Option<serde_json::Value>,
pub context_management: Option<Value>,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all those changes are not needed and unrelated to the main fix. this is just cosmetic

state.accumulated_output.push(approval_event);

// Re-serialize the response body with the new mcp_approval_request event
state.finalize_response_body(body);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since this now mutates the body, you must udated the body access mode to ReadWrite

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MCP approval requests are omitted from the serialized client response

2 participants