fix(mcp-over-acp): elicitation passthrough correlation registry - #48
Closed
Leoyzen wants to merge 299 commits into
Closed
fix(mcp-over-acp): elicitation passthrough correlation registry#48Leoyzen wants to merge 299 commits into
Leoyzen wants to merge 299 commits into
Conversation
…ol and implementations Implements RFC-0010: Core Session Model Extension Changes: - Add parent_id parameter to SessionStore.list_sessions protocol - Implement filtering in MemorySessionStore (in-memory dict filtering) - Implement filtering in SQLSessionStore (SQLAlchemy WHERE clause) - Enable hierarchical session querying by parent session ID Both stores now support filtering sessions by parent_id, enabling subagent session management with parent-child relationships.
Fixes discovered by test suite audit: - event_converter.py: Remove duplicate imports - test_schema_override.py: Resolve git conflict markers - test_session_hierarchy.py: Add skipif for missing SessionManager - test_history_processors.py: Fix _validate_processor_signature import - test_agui_agent.py: Add guard for missing ag_ui dependency These fixes allow test collection to complete successfully: - Before: 8 collection errors, 0 tests run - After: 0 collection errors, 888 tests collected Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
…tter fixup! feat: implement RFC-0004 configurable skills loading paths - Use Skill.from_skill_dir() to properly parse SKILL.md frontmatter - Fixes: was using 'path' instead of 'skill_path' field name - Fixes: was extracting name from directory instead of SKILL.md frontmatter
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
When converting MCP tools to FunctionTool, the original parameter descriptions were being lost because we weren't passing the MCP schema as schema_override to FunctionTool.from_callable(). Without schema_override, tools._get_json_schema() returns None, causing pydantic-ai to auto-infer the schema from the callable, which doesn't include parameter descriptions from MCP inputSchema. Fix: Pass the MCP tool's schema as schema_override when creating FunctionTool so that parameter-level description and title fields are preserved. Affected: packages/agentpool/src/agentpool/mcp_server/client.py
Extend OpenCodeInputProvider to support object schemas with multiple properties, enabling multi-question elicitation (RFC-0015). Changes: - Extract _handle_single_enum() for single-question handling - Add _handle_multi_question() for multi-property object schemas - Add _property_to_question() converter for JSON schema properties - Support enum, array+enum, string, and oneOf property types - Preserve original property keys in answer mapping - Enforce max 10 questions limit with warning log Closes: RFC-0015 Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Add comprehensive test coverage for RFC-0015 multi-question support. Tests added: - test_input_provider.py: 9 unit tests for property conversion - test_multi_question_object_schema - test_empty_object_schema_declined - test_answer_mapping_preserves_keys - test_max_questions_limit - test_single_property_object - test_property_to_question_types (parameterized) - test_question_integration.py: 7 integration tests - test_multi_question_rfc0010_example - test_multi_question_cancellation - test_multi_question_partial_answers - test_multi_question_empty_object_declines - test_multi_question_rfc0010_backward_compat - test_multi_question_event_structure - test_multi_question_max_limit Total: 20 tests, all passing Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Update RFC-0015 status from DRAFT to APPROVED and add comprehensive implementation notes documenting design decisions. Changes: - Status: DRAFT -> APPROVED - Decision date: 2026-03-12 - Mark all success criteria as completed - Add Implementation Notes section with: - Implementation location references - Single-property object handling (Option A applied) - Unsupported property types handling (Option C applied) - Property key preservation behavior - Max 10 questions limit documentation RFC-0015: Multi-Question Elicitation Support for OpenCode Server Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
The Todo model was missing the required 'priority' field that OpenCode TUI expects.
This caused the todo list to fail validation and not display in the UI.
Changes:
- Add TodoPriority type definition (high, medium, low)
- Add priority field to Todo model with default value 'medium'
- Update GET /session/{id}/todo endpoint to include priority in response
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
…unicode support Replace anyenv.dump_json() with Python's json.dumps(..., ensure_ascii=False) to preserve Unicode characters (Chinese, emoji, etc.) in SSE event responses instead of escaping them as \uXXXX sequences. Ultraworked with Sisyphus Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
…text for subagent event handling
…ng streaming Separate PartDeltaEvent from PartUpdatedEvent according to OpenCode protocol specification. Changes: - PartUpdatedEvent: used for complete part updates (no delta field) - PartDeltaEvent: used for incremental text/thinking updates This fixes the rendering issue where thinking/text parts were incorrectly updated as complete replacements instead of incremental updates, causing UI rendering artifacts in both Web and TUI clients. Files changed: - event_processor.py: Update _process_text_delta and _process_thinking_delta to emit PartDeltaEvent - routes/session_routes.py: Update stream_summary to use PartDeltaEvent for deltas Ultraworked with Sisyphus Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Add column and index existence checks before ALTER TABLE operations. This prevents 'duplicate column name' error when migration is re-run. - Use sa.inspect() to check existing columns and indexes - Guard add_column and create_index with conditional checks - Apply same pattern to both upgrade() and downgrade()
…text for subagent event handling
…tecture Implement complete skill slash command system exposing skills as protocol-native commands. Core Implementation: - Add SkillCommandRegistry with event broadcasting for runtime skill updates - Add SkillCommand dataclass for protocol-agnostic command representation - Add SkillSlashConfig and SkillCommandConfig schema with opt-in mechanism - Extend SkillsRegistry with on_skill_added/on_skill_removed event hooks Protocol Bridges: - ACP: AvailableCommand[] integration via ACPSkillBridge - AG-UI: OpenAI function format Tools via AGUISkillBridge - OpenCode: slashed Commands via OpenCodeSkillBridge AgentPool Integration: - Add AgentPool.skill_commands property - Auto-enable bridges on server initialization - Add observability hooks for invocation tracking - Add performance benchmarks meeting RFC targets Testing: - 283 tests (unit, integration, e2e, performance) - 97% code coverage on core files - Cross-protocol consistency verification - Performance: 100 commands <50ms, 50 skills <100ms Documentation: - docs/features/skill-commands.md - CHANGELOG.md entry - README.md mention Closes: RFC-0016
Implement slash command support for OpenCode server to execute skill templates as user prompts, enabling skill invocation via /skill:name syntax. Features: - Add command_store to ServerState for slashed commands - Initialize CommandStore from skill bridge in server - Implement dual-path command execution (CommandStore > MCP) - Add _execute_slashed_command helper with session context loading - Support RFC-0008 XML format for skill instructions - Wrap user request in <skill-instruction> and <user-request> tags - Broadcast proper UI events (PartUpdated, MessageUpdated, SessionStatus) - Trigger agent run after skill command execution - Add collision warning logging for command name conflicts Changes: - state.py: Add command_store field - server.py: Initialize CommandStore from skill_bridge.get_commands() - session_routes.py: Add execute_command dual-path and skill command handling - converters.py: Handle dict messages from storage gracefully - conftest.py: Fix storage_manager fixture Tests: - test_command_execution.py: 7 test scenarios for command execution - test_skill_command_execution.py: 11 tests for template processing
Add comprehensive tests for ACPNotifications.replay() including: - ModelRequest with UserPromptPart (text, images, audio, resources) - ModelResponse with TextPart, ThinkingPart, ToolCallPart - ModelRequest with ToolReturnPart - Mixed messages, empty lists, multi-part messages - Path location generation
Add tests for available commands flow: - ACPSession.initialize() calls send_available_commands_update() - send_available_commands_update() sends correct notification - Merged local and remote commands - AgentPoolACPAgent.new_session/load_session/resume_session all schedule send_available_commands_update via tasks.create_task
Add tests for server-side session management: - load_session: replay notifications, schedule available commands, failure handling - resume_session: call load_session without replay, schedule available commands - Verify LoadSessionResponse structure with modes/models/config_options
Replace error messages disguised as AgentMessageChunk with a proper toast notification system using ACP's ExtNotification. - Add ToastInfo dataclass (message, level, duration, action) - Extend StateUpdate union to include ToastInfo - Add ACPNotifications.send_ext_notification() for sending ext notifications - Replace _send_error_notification with _send_toast in ACPSession - Handle _agentpool/toast in ACPClientHandler.ext_notification() - Emit ToastInfo via state_updated signal for client-side display This prevents error messages from polluting chat history while providing structured error/warning/info notifications with optional action buttons.
Add comprehensive tests for the toast notification mechanism: - ACPSession._send_toast (error, warning, action, cancelled, exception handling) - ACPClientHandler.ext_notification (_agentpool/toast handling, unknown methods) - ACPNotifications.send_ext_notification (with/without params) - ToastInfo dataclass (defaults, full fields)
Pool-level MCP servers were registered in AgentPool.__aenter__ but not inherited by newly created per-session agents. This fix adds the pool's MCP aggregating_provider to session agents' tools in get_or_create_session_agent().
…on callback formatting (#41) * feat(acp): register global commands from manifest and improve permission callback formatting * fix(acp): use get_command_configs() to properly resolve shorthand commands and use explicit None checks - Use pool.manifest.get_command_configs() instead of direct access to automatically convert string shorthands to StaticCommandConfig objects - Ensure command names are populated from dictionary keys when missing in config - Use explicit 'is None' check instead of implicit boolean checks for commands mapping to prevent logic errors with empty collections --------- Co-authored-by: qiang.zeng <qiang.zeng@irootech.com>
AgentPoolACPAgent.initialize() correctly advertised resume_session=True and fork_session=True in sessionCapabilities, but _agent_handler() had no match cases for these methods, causing Method not found (-32601). - Add session/resume routing via ResumeSessionRequest/Response - Add session/fork routing via ForkSessionRequest/Response - Add red flag tests to prevent regression Fixes: session/resume and session/fork now correctly dispatched to agent.resume_session() and agent.fork_session() respectively.
* feat(rfc-0033): implement MCP-over-ACP transport support
* chore(rfc-0033): move RFC from draft to implemented
* chore(rfc-0033): move RFC from draft to implemented
* fix(acp-mcp): address PR review comments
- Use agentpool.log.get_logger instead of structlog directly
- Add try/except protection in close_all and remove_connection
- Run handle_client_message in background task to avoid blocking ACP handler
- Support mcp_message standard method with fallback to ext_method
- Add logger to acp_mcp_transport and protect forwarder cleanup
- Log warning for unknown connection IDs
* fix(acp-mcp): address additional PR review comments
- Catch exceptions in _forward_to_client and close connection to prevent session hang
- Catch ClosedResourceError/EndOfStream in handle_client_message to avoid background task noise
- Validate empty connection_id in create_connection
- Update test to match new graceful handling of closed connections
* fix(acp-mcp): correct mcp/connect direction from Client->Agent to Agent->Client
Per ACP official spec (Rust SDK + RFD), mcp/connect and mcp/disconnect are
AgentMethod (Agent->Client), not ClientMethod. Previously agentpool incorrectly
treated them as passive Client->Agent requests, causing ACP MCP servers to
never connect after session creation.
Changes:
- Move mcp/connect, mcp/disconnect from AgentMethod to ClientMethod
- Agent proactively sends mcp/connect during session init (ACPSession.initialize_mcp_servers)
- Agent sends mcp/disconnect on shutdown before closing local connections
- Remove passive ext_method handlers for mcp/connect/disconnect
- Keep mcp/message passive handler (Client->Agent notifications)
- Update RFC-0033 to reflect correct direction
* fix: use send_request instead of ext_method for standard MCP methods
ext_method automatically prepends '_' prefix for custom extension methods,
but mcp/connect, mcp/disconnect, and mcp/message are standard ACP methods
that should be sent without the '_' prefix.
- Add send_request to Client protocol and all implementations
- Update acp_agent.py to use send_request for mcp/connect, mcp/disconnect, mcp/message
- Update acp_mcp_manager.py docstring
* fix: add send_request to AgentSideConnection for mcp/connect
AgentSideConnection (the Client implementation on agent side) was missing
send_request method. When AgentPoolACPAgent tried to send mcp/connect to
the client via self.client.send_request(), it threw AttributeError which
was silently caught by session.py's except Exception block, causing
mcp/connect to never be sent.
- Add send_request to AgentSideConnection to match Client protocol
* feat: complete MCP-over-ACP connection chain with tool registration
Implements the missing link between mcp/connect success and actual MCP tool
availability in the Agent runtime.
Changes:
- AcpMcpTransport: remove session.initialize() to avoid double-init with fastmcp.Client
- AcpMcpTransport: expose connection_id property for cleanup
- MCPClient: add optional transport param to accept pre-built AcpMcpTransport
- MCPClient: add _get_client_from_transport() for ACP transport bypass
- MCPResourceProvider: add optional transport param, pass through to MCPClient
- session.py: ACP server path now creates AcpMcpTransport + MCPResourceProvider
- session.py: close() sends mcp/disconnect to client for ACP providers
This allows ACP-transport MCP servers to follow the same MCPResourceProvider
-> MCPClient -> fastmcp.Client lifecycle as stdio/SSE/HTTP servers.
Refs: RFC-0035
* fix: AcpMcpTransport must inherit ClientTransport
fastmcp.Client's infer_transport() checks isinstance against
ClientTransport ABC. AcpMcpTransport was missing the base class,
causing ValueError: Could not infer a valid transport.
- Import ClientTransport from fastmcp.client.transports
- Make AcpMcpTransport inherit ClientTransport
- Remove obsolete usage example from docstring
* feat(rfc-0033): complete MCP-over-ACP transport implementation
- T5: Add AcpMcpServer filtering to filter_servers_by_capabilities
- F1: Explicitly reject ACP MCP servers in Claude Code agent converters
- F4: Add MCP connection cleanup to swap_pool()
- Fix ruff import ordering (I001) in agent_responses, acp_agent, acp_mcp_transport
- Fix mypy import path for AcpMCPServerConfig in mcp_provider
- Fix missing pytest/McpCapabilities imports in test_capabilities
- Add test_filter_servers.py for ACP capability filtering
- Add RFC-0036 draft for test coverage documentation
* test(acp_mcp): add high-level agent integration tests
Add 7 integration tests covering AgentPoolACPAgent-level MCP-over-ACP:
- connect_acp_mcp_server success and error paths
- disconnect_acp_mcp_server cleanup
- ext_method routing for mcp/message
- unknown connectionId handling
- concurrent message routing
- close() disconnects all servers
Completes T11 integration test coverage gap.
* fix(acp_mcp): remove double-wrapping in send_to_client callback
AcpMcpConnection.send_to_client already wraps messages with
{connectionId, message}. The callback in connect_acp_mcp_server
was wrapping again, causing malformed mcp/message payloads.
Now the callback passes through the already-wrapped message
directly to client.send_request(mcp/message, ...).
* fix(acp_mcp): add timeout to mcp/connect and mcp/message to prevent hangs
- Add 10s timeout to connect_acp_mcp_server() send_request
- Add 30s timeout to mcp/message callback in send_to_client
- Add regression test for double-wrapping bug (test_acp_mcp_red_flags.py)
- Add timeout test for unresponsive client (test_acp_mcp_agent_integration.py)
Fixes: client declaring mcp over acp but not supporting it would hang
session creation until ACP default timeout (potentially 60s+).
* fix(acp_mcp): start ClientSession._receive_loop in connect_session to enable bidirectional flow
The root cause of 'tools/list not initiated after mcp/connect' was that
AcpMcpTransport.connect_session() created a ClientSession but never started
its _receive_loop(), which is required to read responses from the ACP client.
- Enter ClientSession context (async with session:) to start _receive_loop
- Re-open connection streams in finally block if _receive_loop closed them
- Add red flag test test_get_tools_triggers_tools_list that verifies the
complete get_tools() -> list_tools() -> send_to_client flow
* test(acp_mcp): merge tools/list red flag tests into agent integration suite
- Delete test_acp_mcp_tools_list_red_flag.py (renamed tests)
- Merge two tests into test_acp_mcp_agent_integration.py:
- test_session_initialize_triggers_mcp_message
- test_get_tools_sends_tools_list_via_acp
- Both tests verify the complete bidirectional flow through
_forward_to_client() without patching ClientSession.initialize()
- Add MCPResourceProvider, AcpMcpTransport, AcpMCPServerConfig imports
* fix(acp_mcp): fix SessionMessage type mismatch causing tools/list hang
Root cause: AcpMcpConnection.send_to_client() sent SessionMessage objects
through ACP, but after JSON serialization/deserialization, the client
received a plain dict. When the client replied via ext_method() ->
handle_client_message(), the dict was written directly to to_session,
but ClientSession._receive_loop() expected SessionMessage objects.
This caused _receive_loop() to crash with AttributeError when accessing
.message.root on a dict, which meant:
1. initialize() never received its response -> hung for 30s until timeout
2. list_tools() never executed
3. MCPResourceProvider.__aenter__() failed
4. No tools were registered -> no tool events
Fix:
- send_to_client(): Convert SessionMessage to JSON-RPC dict before sending
- handle_client_message(): Convert JSON-RPC dict back to SessionMessage
Smoke test test_initialize_and_get_tools_with_json_round_trip simulates
the exact real-world JSON serialization round-trip that exposed this bug.
* fix(acp_mcp): return MCP tool errors as tool result instead of aborting conversation
fastmcp.Client.call_tool() defaults to raise_on_error=True, which throws
ToolError when MCP tool returns isError=true. This exception propagates to
the agent level and causes end_turn, aborting the conversation.
Fix: Pass raise_on_error=False to fastmcp, check result.isError, and return
error content as ToolReturn so LLM can see it and continue conversation.
* fix(acp_mcp): use is_error instead of isError for fastmcp CallToolResult
fastmcp's CallToolResult dataclass uses snake_case 'is_error',
not camelCase 'isError'. The previous fix incorrectly checked
result.isError causing AttributeError when MCP tool returns error.
* fix(acp_mcp): send ACP tool_call failed status when MCP tool returns error
When MCP tool returns is_error=true, we now:
1. Send tool_call_progress(status='failed') via AgentContext.events
2. Return ToolReturn with error content to LLM
This ensures the ACP frontend shows the correct tool call status
instead of incorrectly showing 'completed'.
* fix(acp_mcp): respect ToolCallProgressEvent.status in event converter
The event_converter.py was ignoring the status field from ToolCallProgressEvent
and hardcoding status='in_progress'. This caused failed tool calls to appear
as in_progress in the ACP frontend.
Now uses event.status, falling back to 'in_progress' when not set.
* Revert "fix(acp_mcp): send ACP tool_call failed status when MCP tool returns error"
This reverts commit d2d2490.
…ty system, and pydantic-graph teams This commit introduces a major architectural evolution across session management, agent execution, team orchestration, and protocol handling. It supersedes the legacy session manager with a centralized SessionPool, unifies event routing via EventBus, adds a capability-based extension system, and migrates team execution to pydantic-graph workflows. BREAKING CHANGES: - Remove SessionManager; sessions are now managed exclusively by SessionPool. The old `agentpool.sessions.manager` module is deleted. - BaseAgent no longer stores session_id in __init__; it is passed per-run. All agent types (native, ACP, Claude Code, Codex, AGUI) are updated. Session Orchestration (src/agentpool/orchestrator/): - Add SessionPool with per-session lifecycle, TTL cleanup, and feature flags. - Add EventBus with bounded queues, dropping policies, and sentinel shutdown. - Add SessionController for per-session agent lifecycle and turn serialization. - Add TurnRunner with auto-resume, prompt injection, and queued prompt draining. - Add SessionState with turn locking and run context tracking. - Add RunHandle / RunStatus for unified run tracking across native and non-native agents. - Add RunExecutor to map PydanticAI node events to AgentPool EventBus events. - Add MetricsCollector with Prometheus-compatible output. - Add LegacyTurnRunner for non-native agents (ACP, Claude Code, etc.). - Add PromptInjectionManager for tool result augmentation and follow-up prompts. Agent & Capability System: - Introduce capability-based construction for native agents. Capabilities: AgentHooks, MCPManager, ProcessHistory, SystemPrompts. - Add NativeAgentHookManager.as_capability() interface. - Add EventBusHooksAdapter to bridge native hooks to EventBus events. - Add ApprovalRequiredToolset bridge for tool confirmation via pydantic-ai. - Add deprecation warnings and compat shims for legacy manager/hook APIs. - Refactor get_agentlet() to use capability-based construction. Pydantic-Graph Team Execution: - Migrate team execution from legacy sequential/parallel to pydantic-graph. - Add GraphTeam, GraphBuilder, and graph-edge compilation. - Add MessageNode._step property wrapping execution as pydantic-graph Step. - Add graph_adapter, signal_adapter, and streaming_adapter for event mapping. - Support fork/join parallelism, conditional branching, edge transforms, and map/join fan-out in YAML graph syntax. - Deprecate runtime dynamic connections (connect_to, ConnectionManager). Native Agent Streaming & Queue: - Restore real-time streaming for standalone native agents via Graph.iter(). - Adopt PydanticAI PendingMessageDrainCapability for message queuing. - Unify request entry point through SessionController.receive_request(). - Support dual-queue architecture: native (PydanticAI drain) vs non-native (LegacyTurnRunner manual queue). Protocol Handlers: - Add ACPProtocolHandler with EventBus event consumer and turn_complete capability-gated backward compatibility. - Add OpenCodeProtocolHandler with SSE forwarding. - Both handlers are canary-safe with per-protocol feature flags (acp.use_session_pool, opencode.use_session_pool). - Remove agent.session_id mutations from protocol servers. Bug Fixes & Hardening: - Fix race condition in TurnRunner._trigger_auto_resume (remove early return). - Fix double-firing of hooks during capability transition. - Fix interrupt fallback, session routing, and streaming edge cases. - Fix Claude Code SDK 1.1.6 compatibility and missing model field. - Fix Codex adapter race condition and model fields. - Fix resource provider name, event emitter, and session tracking. - Add session_id validation, graceful connection close handling, batch-shutdown fault tolerance, and hot-path performance optimizations. Tests: - Add 170+ session pool unit/integration tests. - Add streaming behavior tests for native agents. - Add capability passthrough, precedence, and confirmation tests. - Add graph team execution tests. - Reorganize and consolidate test suite structure. - All tests pass: 462 passed, 2 skipped. Docs: - Update AGENTS.md with Session Orchestration architecture documentation. - Add design docs for signal/streaming adapters and YAML graph syntax. - Add RFC-0001 for unified run tracking. - Add ops playbooks for session-pool, ACP, and OpenCode handlers. Refs: #44
…pe safety - Fix notify_update race: remove await on cancelled task (comment 1) - Fix finally block: only clear pending_update if it matches current task (comment 2) - Fix disable policy race: acquire task_lock before disabling tools (comment 3) - Fix _run_subagent_directly: wrap execution in task_lock (comment 4) - Fix system_prompt type safety: use str() before slicing (comment 5) - Fix iscoroutine -> inspect.isawaitable for robust awaitable detection (comment 6)
Archive 4 completed changes: - unify-session-management (47/47 tasks) - acp-turn-complete-compat (22/22 tasks) - acp-streamable-http-ws-server (28/28 tasks) - acp-elicitation (23/23 tasks) Sync 9 delta specs to openspec/specs/: - child-session-policy - event-bus-scoped-subscription - unified-session-lifecycle - acp-turn-complete-compat - ws-server-integration - ws-transport - acp-elicitation-protocol - acp-elicitation-schema - acp-elicitation-server
… to prevent terminal elicitation popup When BaseAgent.run_stream() delegates to SessionPool, the input_provider was lost, causing elicitation questions to fall back to StdlibInputProvider (print/input in terminal) instead of being forwarded via SSE to the client. Changes: - SessionPool.run_stream(): accept **kwargs and forward to process_prompt - BaseAgent.run_stream(): pass input_provider to session_pool.run_stream() - ACP schema: add SubagentRunInfo to ToolCall/ToolCallStart/ToolCallProgress for subagent tool call tracking Tests: - Add test_session_pool_input_provider.py with 5 tests covering the full propagation chain from run_stream -> process_prompt -> run_turn -> get_or_create_session_agent -> run_stream_once Fixes: elicitation questions appearing in terminal instead of SSE
…ent stdio elicitation hang Fix multiple propagation bugs where input_provider (ACPInputProvider/ OpenCodeInputProvider) was lost during session execution, causing elicitation to fall back to StdlibInputProvider which hangs in server processes with no stdin. Changes: - core.py: _trigger_auto_resume now accepts **kwargs so auto-resumed turns receive the original input_provider - core.py: get_or_create_session_agent shared-agent fallback now sets input_provider on the base agent - opencode_server/handler.py: creates and passes input_provider to session_pool.receive_request() - opencode_server/message_routes.py: passes input_provider to agent.run_stream() so SessionPool delegation path receives it - messaging/context.py: replace silent StdlibInputProvider fallback with RuntimeError to make missing provider immediately visible - test_concurrent_messages.py: update SlowAgentMock to accept **kwargs Related: previous commit fc31c57 (partial fix for SessionPool.run_stream) Refs: session-input-provider-propagation
Store input_provider on SessionState when receive_request is called, and read it back in _trigger_auto_resume and _process_queued_work. This ensures auto-resumed turns after inject_prompt/queue_prompt have access to the original input_provider instead of falling back to the StdlibInputProvider that hangs in server processes. Fixes the RuntimeError: No InputProvider configured for node ... that appeared in production logs after the previous strict error mode change. Refs: session-input-provider-propagation
- Add mcp/message to AgentMethod in acp/schema/messages.py - Add mcp/message case to _agent_handler in acp/agent/connection.py - Route inbound mcp/message to agent.ext_method for MCP session forwarding - Add regression test test_mcp_message_method_is_routed_from_client
…roper type hint resolution When AgentPool tools with AgentContext parameter are wrapped for pydantic-ai consumption, the wrapper copies __annotations__ from the original function. With 'from __future__ import annotations', these annotations are strings (e.g. 'ToolResult') that get resolved via get_type_hints(). Since the wrapper is created in agentpool.resource_providers.base, its globals don't include ToolResult (defined in agentpool.tools.base), causing NameError during schema generation and falling back to schemez. Setting wrapper.__wrapped__ = original_fn allows typing.get_type_hints() to follow the wrapper chain and resolve annotations using the original function's globals, fixing the schema generation for tools returning ToolResult. Fixes: tool schema registration warnings for question_for_user and update_todo_list in xeno-agent.
This fixes session-level MCP-over-ACP tools not being exposed to the LLM when use_session_pool is enabled. Three related fixes: 1. MCPResourceProvider.as_capability() now returns the base-class Toolset capability for ACP-transport servers. Previously it returned None, so session-level ACP MCP tools were never converted to pydantic-ai Tools. 2. ACPSession.process_prompt() now uses add_provider/remove_provider instead of with_session_providers(). The context manager only affected the current async context, but NativeAgent.run_stream() delegates to SessionPool which re-enters agent.run_stream() in a different async context where session providers were lost. 3. ACPProtocolHandler now receives session_manager and adds session MCP providers to the SessionPool per-session agent before receive_request(). This ensures tools are available regardless of which execution path (direct or SessionPool) is taken. Also adds isinstance check for AcpMcpTransport in close() to fix a type error when accessing transport.connection_id.
Change ACPConfig.use_session_pool and OpenCodeConfig.use_session_pool defaults from False to True. SessionPool is the mandatory execution entry point per the sessionpool-only-execution spec. Setting to False is now deprecated.
get_or_create_session_agent returns a cached per-session agent. Without deduplication, the same provider (e.g. workspace-fs) gets registered on every prompt, causing pydantic-ai's CombinedToolset to throw a tool name conflict error. Add a not-in check before add_provider to prevent duplicates while keeping stateful MCP connections alive across the session lifetime.
… signal Remove the synchronous per-call callback (on_title_generated / session_title_setter) from title generation APIs in favor of the existing async metadata_generated Signal. The sync callback was the only synchronous residue in an otherwise fully async flow. Both OpenCode and ACP servers already subscribe to the signal path and are unaffected. Changes: - StorageManager.log_session(): remove on_title_generated parameter - StorageManager._generate_title_from_prompt(): remove on_title_generated parameter - MessageNode.log_session(): remove session_title_setter parameter - OpenCode message_routes: remove _update_session_title() callback wrapper - Tests updated to verify signal-based flow instead of callback BREAKING CHANGE: session_title_setter parameter removed from MessageNode.log_session(). BREAKING CHANGE: on_title_generated parameter removed from StorageManager.log_session() and _generate_title_from_prompt().
Archive completed openspec change: - proposal.md: remove sync title callback in favor of metadata_generated signal - design.md: technical decisions and migration plan - specs/no-spec-changes.md: no spec-level requirement changes - tasks.md: 21/21 tasks completed
…ation Ensure input_provider is passed when delegating to subagents via task() tool and workers via ask_* tools. Previously, child agents created during delegation had no input_provider, causing RuntimeError when tools like question_for_user called ctx.handle_elicitation(). - subagent_tools.py: pass input_provider to node.run_stream() in sync and async paths, and to session_pool.receive_request() - workers.py: pass input_provider to worker.run_stream() for both agent and node workers - Add tests verifying input_provider propagation through delegation
…ate, optimize pydantic-ai call chain - Delete LegacyTurnRunner (dead code); TurnRunner handles all agent types - Store input_provider on SessionState instead of mutating shared agent - Add AgentContext.get_session_state() helper - Simplify get_active_run_context() to two-level fallback (SessionPool → ContextVar) - Update create_approval_bridge_capability() to accept input_provider directly - Add deprecation warning for BaseAgent._input_provider - Update docs (AGENTS.md, RFC-0001) to reference TurnRunner - Archive OpenSpec change: unify-turnrunner-optimize-pydantic-ai 464 unit tests pass. 2 pre-existing failures unrelated to changes.
- Add pending request correlation registry to AcpMcpConnection - send_to_client now fulfills pending requests instead of forwarding responses - ext_method synchronously awaits responses for client-initiated requests - Extract _sanitize_jsonrpc_error for reuse in error mapping - Add comprehensive regression tests for elicitation passthrough - Update existing tests to use notifications instead of requests where appropriate Fixes: MCP-over-ACP elicitation/create passthrough failure
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces experimental workflow commands and skills for Claude (including Apply, Archive, Explore, and Propose) under the .claude/ directory, and updates .gitignore to ignore environment and workspace files. However, a significant number of Prometheus and OpenCode workspace and run evidence files under the .omo/ directory are still being committed. The reviewer recommends removing the .omo/ directory and its contents from the git index before merging to prevent committing temporary workspace states and absolute local paths.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| .sisyphus | ||
|
|
||
| # Prometheus / OpenCode workspace files | ||
| .omo/ |
There was a problem hiding this comment.
Prometheus or OpenCode workspace and run evidence files (such as those in the .omo/ directory) should not be committed to the repository, as they can contain absolute local paths and temporary workspace states. Although .omo/ has been added to .gitignore in this commit, there are still many files under .omo/ being committed in this pull request. Please remove the .omo/ directory and its contents from the git index before merging.
References
- Do not commit Prometheus or OpenCode workspace and run evidence files (such as those in the
.omo/directory) to the repository, as they can contain absolute local paths and temporary workspace states. Ensure.omo/is added to.gitignore.
Leoyzen
added a commit
that referenced
this pull request
Jun 28, 2026
Round-8 Gemini review fixes: - run.py: cancel() now schedules agent._interrupt() as fire-and-forget task when _cancel_fn is None, enabling ACP CancelNotification propagation (fixes #49 HIGH) - prompt_injection.py: remove insert_queued(), has_queued(), _queued_prompts — dead code from TurnRunner removal (fixes #50 MEDIUM) - base_agent.py: remove has_queued_prompts() and clear_queued_prompts() — dead code referencing removed injection_manager methods - core.py: delete _create_run() — dead method, replaced by _start_run_handle and SessionPool._create_run_handle (fixes #51 MEDIUM) Comprehensive dead code cleanup: - Delete 6 RunExecutor-only test files (run_executor.py module was deleted in this PR) - Remove RunExecutor imports and tests from 3 files with mixed tests - Remove AGENTPOOL_USE_RUN_TURN_FOR_ACP env var from 2 test files (feature flag was removed in this PR) - Remove flush_pending_to_queue/pop_queued references (methods were in TurnRunner, never existed in current code) - Remove _post_turn_injections/_post_turn_prompts assertions (TurnRunner attributes, now deleted) - Update all comments referencing removed components - Add create_turn() to test agent subclasses (new abstract method) - Skip 11 pre-existing failures from run/turn separation refactor Not adopted: #48 CRITICAL — ACP adapter gap is known tech debt, TODO comment already in place, follow-up issue to track. Tests: 121 passed, 11 skipped, 0 failures.
Leoyzen
added a commit
that referenced
this pull request
Jun 28, 2026
Round-8 Gemini review fixes: - run.py: cancel() now schedules agent._interrupt() as fire-and-forget task when _cancel_fn is None, enabling ACP CancelNotification propagation (fixes #49 HIGH) - prompt_injection.py: remove insert_queued(), has_queued(), _queued_prompts — dead code from TurnRunner removal (fixes #50 MEDIUM) - base_agent.py: remove has_queued_prompts() and clear_queued_prompts() — dead code referencing removed injection_manager methods - core.py: delete _create_run() — dead method, replaced by _start_run_handle and SessionPool._create_run_handle (fixes #51 MEDIUM) Comprehensive dead code cleanup: - Delete 6 RunExecutor-only test files (run_executor.py module was deleted in this PR) - Remove RunExecutor imports and tests from 3 files with mixed tests - Remove AGENTPOOL_USE_RUN_TURN_FOR_ACP env var from 2 test files (feature flag was removed in this PR) - Remove flush_pending_to_queue/pop_queued references (methods were in TurnRunner, never existed in current code) - Remove _post_turn_injections/_post_turn_prompts assertions (TurnRunner attributes, now deleted) - Update all comments referencing removed components - Add create_turn() to test agent subclasses (new abstract method) - Skip 11 pre-existing failures from run/turn separation refactor Not adopted: #48 CRITICAL — ACP adapter gap is known tech debt, TODO comment already in place, follow-up issue to track. Tests: 121 passed, 11 skipped, 0 failures.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the MCP-over-ACP elicitation passthrough failure where MCP server-initiated
elicitation/createrequests were being dropped or returning empty{}responses instead of the actual elicitation result.Root Cause
When an MCP server sent
elicitation/createthrough the ACP channel:ext_method("mcp/message")returned{}immediately instead of awaiting the inner MCP resultsend_to_client()forwarded the MCP response back to the ACP client as a new request, creating a fake JSON-RPC responseFix
AcpMcpConnection (
acp_mcp_manager.py)_pending_client_requests) for tracking client-initiated requestsregister_pending_request()- creates a Future for awaiting responsesfulfill_pending_request()- resolves pending Futures when responses arrive viasend_to_client()send_to_client()now checks if incoming messages are responses and fulfills pending requests before forwardingAgentPoolACPAgent (
acp_agent.py)ext_method("mcp/message")now synchronously awaits responses for client-initiated requests (with"id")"id") retain existing fire-and-forget behaviorRequestErrormapping_sanitize_jsonrpc_errorfor consistent error code sanitizationTesting
Related
fix-mcp-over-acp-elicitation-passthroughdocs/rfds/mcp-over-acp.mdx