Skip to content

fix(mcp): preserve assistant messages before tool results#52

Open
rksharma-owg wants to merge 1 commit into
GenAI-Security-Project:mainfrom
rksharma-owg:codex/fix-mcp-tool-message-order
Open

fix(mcp): preserve assistant messages before tool results#52
rksharma-owg wants to merge 1 commit into
GenAI-Security-Project:mainfrom
rksharma-owg:codex/fix-mcp-tool-message-order

Conversation

@rksharma-owg

Copy link
Copy Markdown

Summary

  • append each assistant message containing tool_calls before its tool results
  • preserve tool-call ordering and IDs across successful, failed, and multiple calls
  • add isolated async regression tests for the MCP chat-completion loop

Problem

The MCP mock forwarded tool results without first retaining the assistant message that requested them. A follow-up completion therefore received roles user, tool instead of the protocol-valid user, assistant, tool, which compatible OpenAI endpoints reject.

The focused reproducer exercised the real chat_completions function with mocked transports and observed the missing assistant message. No matching issue, pull request, or branch was open when I audited the repository.

Validation

  • uv run python -m unittest discover -s tests -v — 3 passed
  • uv run black --check app/ client/ tests/
  • uv run isort --check-only app/ client/ tests/
  • uv run python -m compileall -q app client tests
  • git diff --check
  • uv run mypy app/ client/ — retains the same 7 pre-existing errors reproduced on pristine main; this patch adds none

The repository's make test target was not run because it pulls the 20B Ollama model and provisions the full Podman stack; the changed loop is covered without external services by the new regression suite.

AI assistance

AI assistance was used during the audit and test drafting. I reproduced the defect against the real function and reviewed and ran every submitted change locally.

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.

1 participant