Skip to content

fix(mcp-over-acp): elicitation passthrough correlation registry - #116

Closed
Million-mo wants to merge 2 commits into
mainfrom
develop/agentic
Closed

fix(mcp-over-acp): elicitation passthrough correlation registry#116
Million-mo wants to merge 2 commits into
mainfrom
develop/agentic

Conversation

@Million-mo

Copy link
Copy Markdown
Owner

Summary

Fixes the MCP-over-ACP elicitation passthrough failure where MCP server-initiated elicitation/create requests were being dropped or returning empty {} responses instead of the actual elicitation result.

Root Cause

When an MCP server sent elicitation/create through the ACP channel:

  1. ext_method("mcp/message") returned {} immediately instead of awaiting the inner MCP result
  2. send_to_client() forwarded the MCP response back to the ACP client as a new request, creating a fake JSON-RPC response

Fix

AcpMcpConnection (acp_mcp_manager.py)

  • Added correlation registry (_pending_client_requests) for tracking client-initiated requests
  • register_pending_request() - creates a Future for awaiting responses
  • fulfill_pending_request() - resolves pending Futures when responses arrive via send_to_client()
  • send_to_client() now checks if incoming messages are responses and fulfills pending requests before forwarding
  • Unmatched responses are dropped with a warning instead of being forwarded as spurious requests

AgentPoolACPAgent (acp_agent.py)

  • ext_method("mcp/message") now synchronously awaits responses for client-initiated requests (with "id")
  • Notifications (no "id") retain existing fire-and-forget behavior
  • Proper timeout and error handling with RequestError mapping
  • Extracted _sanitize_jsonrpc_error for consistent error code sanitization

Testing

  • Added 6 red-flag regression tests covering: elicitation passthrough, timeout handling, unmatched response dropping, duplicate response handling
  • Added 9 end-to-end regression tests covering: concurrent requests, timeout, duplicate ID rejection, agent-initiated path, notification handling, connection close cleanup
  • Updated existing tests to use notifications instead of requests where fire-and-forget behavior is expected
  • All 58 MCP tests pass

Related

  • OpenSpec change: fix-mcp-over-acp-elicitation-passthrough
  • RFD: docs/rfds/mcp-over-acp.mdx

备份迁移自 SRC-PR#48 · 作者 @Leoyzen · 创建于 2026-06-08T03:03:34Z · head=develop/agentic base=main
源状态: closed

Million-mo and others added 2 commits July 13, 2026 09:35
* feat(acp): batch SessionUpdate delivery during replay

Refactor ACPNotifications.replay() from sequential per-update
session/update notifications to collect-then-batch-send pattern:

- Add _collect_request_updates/_collect_response_updates as pure
  conversion methods returning list[SessionUpdate] (no I/O)
- Add send_batch_update() that uses ext_notification
  _batch_session_updates when client supports it, falls back to
  sequential session/update otherwise
- Add notification_batch_size (default 20) and notification_flush_interval
  (default 0.0) constructor params to ACPNotifications
- Add set_batch_support() for capability-based opt-in
- Wire batch support detection into ACPSession.__post_init__ via
  client_capabilities.field_meta
- Keep _replay_request/_replay_response as thin wrappers for backward
  compatibility
- Add 6 new tests covering batch mode, fallback, ordering, custom size,
  pure collector, and empty messages
- Add benchmark script comparing batch vs sequential

OpenSpec change: acp-notification-batching

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

* refactor(acp): address PR #68 review feedback

- Remove  parameter (YAGNI — no real use case)
- Move benchmark script from tests/ to scripts/ to avoid pytest collection
- Revert unrelated  CI fix from test_from_config_capabilities_not_duplicated
- Add defensive debug log when _tool_call_inputs cache misses on ToolReturnPart

* fix(test): restore monkeypatch.setenv for OPENAI_API_KEY in test_from_config_capabilities_not_duplicated

CI environment has no OPENAI_API_KEY set; the setenv is required for
the test that creates a NativeAgentConfig with model=openai:gpt-4o-mini.

---------

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
…sage API (#149)

* fix(opencode): update tool input from ToolCallProgressEvent

When ToolCallStartEvent arrives before tool arguments are fully
streamed, ctx stores an empty dict as tool_input. Subsequent
ToolCallProgressEvent carries the complete event_tool_input but
_process_tool_progress ignored it, reading the stale empty value
from ctx instead. This caused tool call parameters to always
appear empty in the session message API.

Fix: update ctx with event_tool_input when it differs from the
stored value, before constructing the tool state.

Closes #148

* fix(opencode): convert event_tool_input to camelCase before comparison

Address Gemini review feedback: comparing raw snake_case
event_tool_input with camelCase current_input always evaluates
to True when keys contain underscores, causing redundant
ctx.set_tool_input calls on every progress event.

Convert to camelCase first, then compare.
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