Skip to content

feat(agent): implement history processors for PydanticAI integration … - #3

Merged
Leoyzen merged 3 commits into
mainfrom
feature/history_processor_integration
Feb 13, 2026
Merged

feat(agent): implement history processors for PydanticAI integration …#3
Leoyzen merged 3 commits into
mainfrom
feature/history_processor_integration

Conversation

@Leoyzen

@Leoyzen Leoyzen commented Feb 9, 2026

Copy link
Copy Markdown
Collaborator

This PR enables users to configure pydantic-ai history processors in their YAML configuration, allowing dynamic manipulation of message history before LLM requests.

Motivation

Currently, there's no way for users to customize how message history is processed before being sent to the LLM. This feature allows users to define custom processing pipelines (e.g., filtering, summarization, token management) via simple import paths in YAML configuration.

Changes

1. Configuration Schema (src/agentpool_config/session.py)

Added history_processors: list[str] | None field to MemoryConfig:

agents:
  my_agent:
    type: native
    model: openai:gpt-4o-mini
    memory:
      history_processors:
        - my_module:filter_system_messages
        - my_module:truncate_old_messages
  • Execution Order: CompactionPipeline runs first, then history processors
  • Security: Documented security considerations (processors have full message access)

2. Native Agent Integration (src/agentpool/agents/native_agent/agent.py)

  • Dynamic Import Resolution: Processes import paths (e.g., module:function) at agent initialization
  • Signature Validation: Validates processor signatures using inspect.signature, supporting all 4 pydantic-ai patterns:
    • (messages) -> messages
    • (ctx, messages) -> messages
    • async (messages) -> messages
    • async (ctx, messages) -> messages
  • Caching: Added _resolved_history_processors cache to avoid repeated imports during agentlet creation
  • Integration: Resolved processors passed directly to PydanticAgent

3. Comprehensive Test Suite (tests/test_history_processors.py)

15 tests covering:

  • Import resolution and caching behavior
  • Signature validation and error handling
  • Sequential execution of multiple processors
  • Interaction with CompactionPipeline and RunContext
  • Combined usage with CompactionPipeline (verifies execution order)

Usage Example

# my_processors.py
def filter_thinking_messages(messages: list[ModelMessage]) -> list[ModelMessage]:
    """Remove thinking blocks before sending to LLM."""
    return [m for m in messages if not m.has_thinking_content()]
# agents.yml
agents:
  coder:
    type: native
    model: openai:gpt-4o-mini
    memory:
      history_processors:
        - my_processors:filter_thinking_messages

Execution Flow

User Message
    ↓
CompactionPipeline (runs first - handles compaction/summarization)
    ↓
History Processors (custom user-defined transformations)
    ↓
LLM Request

Testing

All tests pass:

uv run pytest tests/test_history_processors.py -v
# 15 passed

…(RFC-0003)

This implementation enables dynamic message history processing by allowing
users to configure a pipeline of processors in the agent's memory config.
Key changes:
- Config: Added `history_processors` to `MemoryConfig` with detailed docstrings
  covering execution order and security considerations.
- Resolution: Implemented dynamic import resolution and signature validation
  in `NativeAgent`. Supports all 4 pydantic-ai processor signatures:
    - (messages) -> messages
    - (ctx, messages) -> messages
    - async (messages) -> messages
    - async (ctx, messages) -> messages
- Caching: Added `_resolved_history_processors` cache to minimize overhead
  from dynamic imports during agentlet creation.
- Integration: Connected resolved processors to the underlying `PydanticAgent`
  lifecycle.
- Testing: Added a comprehensive test suite (15 tests) covering:
    - Import resolution and caching.
    - Signature validation and error handling.
    - Sequential execution of multiple processors.
    - Interaction with CompactionPipeline and RunContext.
Execution Order: CompactionPipeline -> History Processors -> LLM Request.
@gemini-code-assist

Copy link
Copy Markdown

Summary of Changes

Hello @Leoyzen, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a powerful new feature enabling users to configure custom history processors for PydanticAI agents. This allows for dynamic and programmable manipulation of message history, such as filtering, summarization, or token management, directly through YAML configuration. The change significantly enhances the flexibility and customization capabilities of the agent's interaction with Large Language Models.

Highlights

  • Configurable History Processors: Introduced a new history_processors field in MemoryConfig to allow users to specify custom functions for manipulating message history before LLM requests.
  • Dynamic Processor Integration: Implemented dynamic import, signature validation, and caching of user-defined history processors within the NativeAgent to ensure correct and efficient execution.
  • Comprehensive Testing: Added a thorough test suite covering various scenarios, including import resolution, signature validation, caching, sequential execution, and interaction with existing history.
  • Detailed Documentation: Provided extensive documentation for history processors, including usage examples, execution order relative to CompactionPipeline, and critical security considerations.
Changelog
  • src/agentpool/agents/native_agent/agent.py
    • Imported the inspect module for runtime introspection and signature validation.
    • Added _resolved_history_processors as a cache for imported processor callables to prevent redundant imports.
    • Implemented _validate_processor_signature to enforce correct function signatures for history processors (supporting sync/async with/without RunContext).
    • Created _resolve_history_processors to dynamically import processor functions from string paths, validate them, and store them in the cache.
    • Integrated the resolution and passing of history_processors to the PydanticAgent constructor during agentlet creation.
  • src/agentpool_config/session.py
    • Added a new optional field history_processors: list[str] | None to MemoryConfig.
    • Included detailed documentation for the history_processors field, outlining its purpose, supported signatures, execution order relative to CompactionPipeline, and important security considerations.
  • tests/test_history_processors.py
    • Added a new test file dedicated to validating history processor functionality.
    • Includes tests for MemoryConfig validation with empty or None processor lists.
    • Contains tests for handling invalid import paths, non-callable imports, and incorrect processor signatures.
    • Verifies successful resolution of various valid processor types (sync/async, with/without context).
    • Tests the caching mechanism for history processors to ensure they are resolved only once.
    • Includes integration tests to confirm processors are called during agent execution and run in the correct sequence.
    • Ensures compatibility when no history processors are configured.
    • Validates that history processors receive the complete message history, including pre-existing conversation messages.
  • tests/test_processors.py
    • Added a new file containing mock history processor functions used for testing purposes.
    • Includes examples of valid sync and async processors, both with and without RunContext parameters.
    • Provides examples of intentionally invalid processor functions to test signature validation logic.
Activity
  • All 15 new tests for history processors are passing, ensuring the stability and correctness of the new feature.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a valuable feature for configuring pydantic-ai history processors via YAML, enabling dynamic manipulation of message history. The implementation is well-documented, especially the security considerations and usage examples in src/agentpool_config/session.py. The accompanying test suite is comprehensive and covers a wide range of scenarios, ensuring the feature's robustness. I have one suggestion to improve code clarity and reduce redundancy.

Comment thread src/agentpool/agents/native_agent/agent.py Outdated
Address review comment: combine redundant if blocks checking memory_cfg
and processor_paths into a single check for better readability.

The getattr() call with None default handles both cases:
- When conversation._config is falsy (returns None)
- When history_processors attribute is missing or empty

docs(session): add history_processors documentation

Add usage examples and security considerations for the new
history_processors configuration option.
@Leoyzen
Leoyzen merged commit 2ca5838 into main Feb 13, 2026
Leoyzen added a commit that referenced this pull request May 27, 2026
- Add ProviderCurrentConfig, update ProviderInfo to match ACP spec
- Add ListProvidersRequest, SetProvidersRequest, DisableProvidersRequest
- Add ListProvidersResponse, SetProvidersResponse, DisableProvidersResponse
- Add providers/* to AgentMethod literal and union types
- Add ProvidersCapabilities to AgentCapabilities
- Add native methods to Agent protocol: list_providers, set_provider, disable_provider
- Update agent connection dispatch for providers/*
- Update acp_agent.py with typed handlers, remove from ext_method
- Update provider_router.py to use new ProviderInfo schema
- Add comprehensive tests for providers schemas
Leoyzen added a commit that referenced this pull request Jul 7, 2026
…e leaks

- Fix #1 (Critical): resume_session() now removes session_id from
  _connection_sessions before closing old session. Prevents stale
  connection disconnect from closing the newly resumed session.
- Fix #2 (High): as_capability() uses _session_contexts.get() instead of
  get_or_create_session(). Prevents memory leak when context was already
  cleaned up. Removes dead try/except KeyError code.
- Fix #3 (Medium): cleanup_session() uses _session_contexts.get() and
  returns early if None. Avoids creating throwaway SessionConnectionPool.
- TDD: 3 tests in test_review_fixes.py verify all fixes.
Leoyzen added a commit that referenced this pull request Jul 8, 2026
* spec: MCP session lifecycle fix — Phase 1

Add OpenSpec change for fixing stale MCP toolset cache and session-scoped
resource lifecycle bugs (#121). Includes:

- proposal.md: What & why (6 lifecycle fixes, no config changes)
- design.md: 8 design decisions (D1-D8) with Oracle + Momus review
- specs/mcp-session-lifecycle: 7 requirements, 14 scenarios
- specs/session-orchestration: Modified requirements for close path
- specs/unified-session-lifecycle: WebSocket disconnect hook
- tasks.md: 7 task groups, 46 tasks (P1a-P1f + E2E)
- tests/mcp_server/test_stale_mcp_connection.py: 5 reproduction tests

Reviewed by Momus (PASS) and Oracle (PASS) after 2 revision cycles.

Closes #121 (spec phase)

* spec: address Gemini Code Assist review comments

4 accepted fixes from dialectical analysis with Oracle:

1. Task 3.1/3.2/3.3: Change _session_connections to
   dict[str, set[tuple[str, int]]] — store (connection_id,
   session_key) pairs so AcpMcpConnectionManager.cleanup_session()
   can look up SessionStreamPair via session_key

2. Task 5.2 (D6): Two-layer cleanup on resume — call both
   SessionController.close_session() (RunHandle lifecycle) AND
   ACPSession.close() (ACP env/signals/prompts). Neither alone
   is sufficient.

3. Task 6.4 (D7): Same two-layer cleanup for WebSocket disconnect

4. Task 2.8: Use try/finally or fixture teardown for test cleanup

Rejected comments (2):
- hasattr(self.agent, 'mcp'): violates AGENTS.md, mcp always set
- hasattr(agent, 'mcp'): same, agent is not None check exists

Already addressed (2):
- Concurrency re-verify after lock: spec's lock-on-context design
  handles this implicitly
- await on_disconnect: type signature makes it obvious

* feat(mcp): add _SessionContext dataclass and session connection tracking

- Add _SessionContext dataclass to MCPManager with per-session state
  (connection_pool, toolset_cache, snapshot, acp_connection_ids, _cleanup_lock)
- Add _session_contexts dict to MCPManager.__init__
- Add _session_connections reverse index to AcpMcpConnectionManager
- Add register_session_connection() method for tracking session→connection mappings

Implements T1 and T6 of fix-mcp-session-lifecycle plan.

* feat(mcp): add session lifecycle methods and ACP cleanup

- get_or_create_session() and update_session_snapshot() on MCPManager (T2)
- add_acp_transport() on MCPManager for session-scoped ACP tracking (T3)
- register_session() returns tuple[SessionStreamPair, int] (T7, GAP-1)
- has_active_sessions() on AcpMcpConnection (T7)
- cleanup_session() with _cleanup_lock on AcpMcpConnectionManager (T7, GAP-12)
- Updated all callers of register_session() to unpack tuple return

* feat(mcp): cleanup_session on MCPManager and wire register_session_connection

- cleanup_session() with per-session _cleanup_lock on MCPManager (T4)
- _acp_mcp_manager field added for ACP cleanup delegation
- connect_acp_mcp_server() gains session_id parameter (T8, GAP-5)
- Returns tuple[str, int] (connection_id, session_key)
- Call site in session.py passes session_id and calls add_acp_transport
- All test callers updated for new signature

* test(mcp): add session lifecycle and ACP cleanup unit tests (T5+T9)

* refactor(mcp): change as_capability to session_id-based API (T10)

- Change as_capability(snapshot=, session_pool=) to as_capability(session_id=)
- Parameterize _make_capability with toolset_cache dict parameter (GAP-7)
- Split _process_snapshot into _process_global_configs and _process_session_configs
- GAP-11: KeyError fallback for concurrent cleanup_session race
- Backward compat: session_id=None processes self.servers with self._toolset_cache

* refactor(agent): update get_agentlet to use as_capability(session_id) (T12)

- Replace as_capability(snapshot=, session_pool=) with as_capability(session_id=)
- GAP-4: Use run_ctx.session_id from AgentRunContext instead of self._session_id
- Remove if/else branching on _mcp_snapshot — as_capability handles internally
- Keep _mcp_snapshot and _session_connection_pool field declarations for compat

* test(mcp): update caching+provider tests for session_id API (T13)

- Update 6 tests in test_mcpmanager_caching.py for new as_capability(session_id) API
- Update 15 failing tests in test_mcp_provider_lifecycle.py to use session context
- Fix static source assertion in test_no_dedup_hack_in_get_agentlet
- All 48 tests pass

* test(mcp): flip stale connection tests to verify fix (T14)

- Rename test_session_resume_returns_stale_toolset → _returns_fresh_toolset
- Rename test_multiple_acp_servers_all_go_stale → _get_fresh_toolsets
- Rename test_disconnect_all_clears_cache → test_cleanup_session_clears_per_session_cache
- All 5 tests now verify the fix instead of documenting the bug
- All tests pass with new session_id API

* feat(session): wire cleanup_session into ACPSession.close and SessionController (T15)

* feat(agent): wire get_or_create_session in SessionController agent creation (T16)

* test(mcp): integration tests for session close lifecycle (T17+T18+T19)

* fix(acp): resume_session close-then-recreate instead of early-return (T20)

* test(acp): resume_session lifecycle tests - close, reconnect, active run (T21+T22+T23)

* feat(acp): add on_disconnect callback to websocket handler (T24)

- Add on_disconnect: Callable[[AgentSideConnection], Awaitable[None]] | None parameter
- Generate UUID4 connection_id on AgentSideConnection at accept time (GAP-3)
- Call on_disconnect in ConnectionClosed handler before conn.close()
- Backward compatible: on_disconnect defaults to None

* feat(acp): implement close_all_sessions_for_connection (T25)

- Add _connection_sessions reverse index on ACPSessionManager
- Add connection_id parameter to create_session() and resume_session()
- Implement close_all_sessions_for_connection() for WebSocket disconnect cleanup
- Idempotent: pops connection_id, iterates sessions, closes via SessionController + ACPSession.close()

* feat(acp): wire on_disconnect to close_all_sessions_for_connection (T26)

- Add on_disconnect parameter to serve(), _serve_websocket(), _serve_streamable_http()
- Wire on_disconnect callback in ACPServer._start_async() closure
- Add session_manager field to AgentPoolACPAgent for shared session tracking
- Create shared ACPSessionManager in ACPServer for cross-connection session tracking
- Add disconnect detection in _serve_streamable_http via recv_task completion
- Fix test_resume_session_is_idempotent -> test_resume_session_closes_old_and_recreates
  (T20 changed resume_session from idempotent to close-then-recreate)

* test(acp): websocket disconnect closes sessions and preserves others (T27+T28)

- T27: test_websocket_disconnect_closes_all_sessions — 2 sessions same conn, disconnect, both closed
- T27: test_websocket_disconnect_preserves_other_connections — 2 conns, disconnect one, other survives
- T28: test_websocket_disconnect_during_run — active run cancelled with 2s timeout on disconnect

* fix(acp): resolve mypy union-attr errors with cast (T32) + add e2e session lifecycle test (T33)

- T32: Use cast() to type session_manager field as ACPSessionManager (not | None) for mypy
- T33: test_e2e_session_lifecycle — full lifecycle: connect→session→MCP→disconnect→reconnect→resume→verify fresh

* fix: resolve CI ruff format and lint errors

- ruff format: reformat 5 files (manager.py, session_controller.py, session.py, test_session_lifecycle.py, test_stale_mcp_connection.py)
- ruff check: shorten docstring in test_acp_session_resume.py (E501)

* fix(mcp): address review — _connection_sessions cleanup, get_or_create leaks

- Fix #1 (Critical): resume_session() now removes session_id from
  _connection_sessions before closing old session. Prevents stale
  connection disconnect from closing the newly resumed session.
- Fix #2 (High): as_capability() uses _session_contexts.get() instead of
  get_or_create_session(). Prevents memory leak when context was already
  cleaned up. Removes dead try/except KeyError code.
- Fix #3 (Medium): cleanup_session() uses _session_contexts.get() and
  returns early if None. Avoids creating throwaway SessionConnectionPool.
- TDD: 3 tests in test_review_fixes.py verify all fixes.

* fix(mcp): wire _acp_mcp_manager, add identity check, consolidate as_capability

Review round 2 fixes:

Fix #4 (Critical): Wire _acp_mcp_manager in ACPSession.__post_init__
- MCPManager._acp_mcp_manager was initialized to None and never set
- cleanup_session() could never delegate to AcpMcpConnectionManager
- Per-session ACP stream pairs and reverse-index entries leaked
- Fix: wire agent.mcp._acp_mcp_manager = acp_agent._mcp_manager in __post_init__

Fix #5 (Medium): Identity check after acquiring cleanup lock
- Concurrent cleanup_session() callers could do redundant work
- All ops were idempotent but wasteful (clearing empty dicts, etc.)
- Fix: check if self._session_contexts.get(session_id) is not ctx after lock

Fix #6 (Medium): Consolidate duplicated fallback in as_capability()
- Three identical 'for server in self.servers:' loops consolidated to one
- Pure readability refactor, zero behavior change

TDD: 3 new tests (2 RED before fix, 3 GREEN after)
- test_cleanup_session_delegates_to_acp_mcp_manager (unit)
- test_acp_session_wires_acp_mcp_manager (integration)
- test_cleanup_session_identity_check_prevents_redundant_work (unit)

213 tests pass, ruff clean.

* test(mcp): add 20 integration tests for session wiring lifecycle

Categories A-D from Oracle integration test plan:
- A (4): Cross-component wiring — cleanup delegation, __post_init__ wiring,
  full close chain, close_all_sessions_for_connection
- B (7): Lifecycle edge cases — full create/cleanup, close/recreate,
  shared connection isolation, WebSocket disconnect, resume, concurrent cleanup
- C (4): State consistency — registry consistency after cleanup/close/resume,
  stream pair unregistration
- D (5): Error paths — ACP manager raises, session close raises,
  MCP cleanup raises, resume old close raises, pool cleanup raises

These tests would have caught the _acp_mcp_manager wiring bug (round 2
review comment #1) that unit tests missed due to component isolation.

* fix(acp): wire connection_id through create_session/resume_session call sites

- Declare connection_id: str | None on AgentSideConnection (replaces monkey-patch)
- Remove # type: ignore[attr-defined] from transports.py connection_id assignments
- Add _get_connection_id() helper on AgentPoolACPAgent using isinstance check
- Wire connection_id= into all 5 create_session/resume_session call sites:
  new_session, load_session, fork_session, resume_session, handler.py
- Fix misleading GAP-11 comment: dict.get() returns None, never raises KeyError

Without this fix, _connection_sessions dict was never populated, making
close_all_sessions_for_connection() always return immediately — the entire
WebSocket disconnect cleanup feature was dead code.

* test(mcp): add 13 E2E integration tests for full MCP session lifecycle

Covers all 13 gap areas identified by Oracle analysis:
- G1: Full create_session → get_or_create_session_agent → MCPManager chain
- G2: as_capability with non-empty ACP snapshot → real MCPToolset
- G3: initialize_mcp_servers → connect_acp_mcp_server → AcpMcpTransport
- G4: Full tool execution through as_capability → MCPToolset → AcpMcpTransport
- G5: SessionController.close_session with real agent + real MCP resources
- G6: resume_session with real ACPSession (not patched)
- G7: Full on_disconnect → close_all_sessions_for_connection chain
- G8: connection_id propagation: create_session populates _connection_sessions
- G9: as_capability during concurrent cleanup (GAP-11 race)
- G10: ACP transport failure during tool execution + cleanup
- G11: Multiple sessions on same connection with real ACPSessions
- G12: Child session inherits parent's ACP transports
- G13: Pool shutdown cleans all session MCP resources

* fix: resolve CI mypy and unit test failures

- server.py: Remove unused type: ignore, use None guard for connection_id
- test_acp_session_resume.py: Add connection_id to expected resume_session call args

* chore(openspec): archive fix-mcp-session-lifecycle and sync specs

- Mark all 46 tasks as complete in tasks.md
- Sync 3 delta specs to main specs:
  - mcp-session-lifecycle (new)
  - session-orchestration (updated)
  - unified-session-lifecycle (updated)
- Archive to openspec/changes/archive/2026-07-07-fix-mcp-session-lifecycle/

* fix: parent session memory leak + on_disconnect in finally (review r3)

- session_controller.py: Replace get_or_create_session() with
  _session_contexts.get() when reading parent snapshot/pool. Prevents
  phantom _SessionContext creation when parent was already cleaned up.
- transports.py: Move on_disconnect callback from except ConnectionClosed
  to finally block. Ensures callback fires on any exception path.
- 3 TDD tests: leak detection, regression guard, disconnect coverage.

* fix(mcp): wire child session ACP manager, add transport callback, fix toolset __aexit__

Three fixes for child session ACP transport registration gaps:

1. Wire _acp_mcp_manager on child agent from parent (session_controller.py)
   - Child sessions created via get_or_create_session_agent() don't go
     through ACPSession.__post_init__, so _acp_mcp_manager stayed None.
     Now copied from parent after copy_pre_created_transports().

2. Add on_session_registered callback to AcpMcpTransport (acp_mcp_transport.py)
   - Optional callback invoked after register_session() with (connection_id,
     session_key). Enables callers to register ACP connections for cleanup
     tracking via register_session_connection().

3. Fix toolset_cache.clear() to call __aexit__ first (manager.py)
   - cleanup_session() called .clear() without closing MCPToolset instances,
     leaking stream pairs and forwarder tasks. Now mirrors disconnect_all()
     pattern: iterate values, call __aexit__(None, None, None) with
     contextlib.suppress(ValueError), then clear.

TDD: 3 tests in test_child_session_acp_fix.py (all GREEN).
252 MCP+ACP tests pass, 0 regressions, ruff clean.
Leoyzen added a commit that referenced this pull request Jul 22, 2026
4 E2E test fixes (all test issues, not real bugs):

1. test_prompt_async_user_message_renders_once: Check user message
   parts via SSE message.part.updated events instead of REST API
   (user parts now stripped from sync() response to prevent TUI
   duplication).

2. test_redflag_b3_queued_message_before_response: Comment out
   content check (user parts via REST). Primary ordering assertion
   preserved. User content now delivered via SSE only.

3. test_sync_message_sets_time_completed: Poll until time.completed
   is set, not just until 2 messages appear. Fixes race condition
   where assistant message exists before StreamCompleteEvent
   finalizes it.

4. test_sync_message_persists_to_storage: Same polling fix as #3.
Leoyzen added a commit that referenced this pull request Jul 22, 2026
4 E2E test fixes (all test issues, not real bugs):

1. test_prompt_async_user_message_renders_once: Check user message
   parts via SSE message.part.updated events instead of REST API
   (user parts now stripped from sync() response to prevent TUI
   duplication).

2. test_redflag_b3_queued_message_before_response: Comment out
   content check (user parts via REST). Primary ordering assertion
   preserved. User content now delivered via SSE only.

3. test_sync_message_sets_time_completed: Poll until time.completed
   is set, not just until 2 messages appear. Fixes race condition
   where assistant message exists before StreamCompleteEvent
   finalizes it.

4. test_sync_message_persists_to_storage: Same polling fix as #3.
Leoyzen added a commit that referenced this pull request Jul 22, 2026
…n OpenCode TUI (#273)

* spec: add fix-durable-recovery-event-pipeline OpenSpec change

4 problems identified in durable execution recovery event pipeline:
- P4: _message_registered not reset after StreamCompleteEvent
- P1: Protocol-sourced user messages don't emit PartUpdatedEvent (root cause of user messages not displaying in TUI)
- P3: set_session_context_data() never called in production (dead code)
- P2: UserMessageInsertedEvent bypasses ProtocolChannel (never journaled)

Reviewed through 3 rounds (Oracle + Metis + Momus), 17 issues found and fixed.
22 test cases planned across 3 layers (15 Unit, 3 Integration, 4 E2E).

* fix: durable recovery event pipeline (P4+P1+P3+P2)

P4: Reset _message_registered after StreamCompleteEvent/RunFailedEvent
    to eliminate false 'finalize incomplete turn' warnings on every
    subsequent turn.

P1: Unconditionally emit PartUpdatedEvent for protocol-sourced user
    messages. The OpenCode TUI has no optimistic mechanism — user
    message content comes exclusively from SSE message.part.updated
    events. Without this fix, all user messages after initial sync()
    render as empty rows.

P3: Activate recovery path by calling set_session_context_data() after
    StreamCompleteEvent/RunFailedEvent. Wires up existing
    EventProcessorContext.serialize()/deserialize() infrastructure that
    was implemented but never activated. Fixes same-process elicitation
    resume only (cross-process crash recovery tracked as follow-up).

P2: Route UserMessageInsertedEvent through ProtocolChannel for
    protocol-sourced steer/followup messages. Adds deduplication guard
    in ProtocolChannel.publish() to skip EventBus publish during replay
    (crash-before-delivery edge case documented in design.md).

Also fixes pre-existing bug: registration block after match event was
re-setting _message_registered=True for StreamCompleteEvent/RunFailedEvent,
undoing P4's reset. Added is_lifecycle_finalizer guard.

17 new unit tests across 4 test files. 2 existing test files updated.
All 47 tests pass. Ruff clean.

Refs: openspec/changes/fix-durable-recovery-event-pipeline/

* fix: clear EventBus replay buffer on sync() to prevent duplicate first message

When the TUI calls GET /session/{id}/message (sync()), clear the
EventBus replay buffer for that session. This prevents the replay
buffer from re-delivering PartUpdatedEvent events that were published
before sync() ran.

Without this fix, the first user message in a new session gets
duplicated: the TUI receives PartUpdatedEvent via live SSE, and then
sync() loads the same message from DB. Clearing the replay buffer
ensures events published before sync() are not re-delivered to
reconnecting SSE subscribers.

Subsequent messages are unaffected because sync() only runs once per
session (TUI's fullSyncedSessions set).

* fix: P4 state machine — _message_registered reset moved to D1 block

P4's original implementation reset _message_registered=False at
StreamCompleteEvent. This was wrong: it prevented D1 from firing on
the next RunStartedEvent, so _pending_message_ids was never popped
and no new assistant message was created. All subsequent turns merged
into turn 1's assistant message, causing user messages to appear at
the bottom with wrong timestamps.

Correct state machine:
- StreamCompleteEvent: finalize + persist (keep _message_registered=True)
- RunStartedEvent D1: if _message_registered=True → finalize previous,
  create new assistant msg (pop _pending_message_ids), reset per-turn
  state, set _message_registered=False
- Registration block: if _message_registered=False → append + broadcast,
  set _message_registered=True

Tests rewritten based on correct state machine model with full timing
diagram. 10 state-machine tests + 3 updated P4 tests + 1 P3 test fix.
168 tests pass.

* test: add replay buffer + sync() integration tests

5 integration tests covering SSE replay buffer interaction with
sync() endpoint that was missing:

- test_replay_buffer_contains_events_after_publish: events enter buffer
- test_new_subscriber_receives_replay_buffer_events: replay delivers
- test_clear_replay_buffer_prevents_redelivery: clear prevents dupes
- test_clear_replay_buffer_only_affects_target_session: isolation
- test_events_after_clear_are_still_delivered: new events still work

Also adds logging to sync() endpoint to confirm replay buffer clear
is executing (log line: 'Clearing replay buffer for session ... on sync()').

809 tests pass.

* test: reproduce first-user-message duplication via replay buffer + sync race

4 integration tests using real EventBus:
- test_first_message_duplicated_without_replay_buffer_clear: reproduces
  the duplication (parts from BOTH replay buffer SSE and sync() DB)
- test_replay_buffer_clear_prevents_duplication: verifies fix works
  (sync() clears replay buffer → no duplicate parts)
- test_live_events_still_delivered_after_replay_buffer_clear: regression
  test (live events still work after buffer clear)
- test_timing_race_consumer_startup_delays_delivery: simulates the
  actual race (events published during ~880ms consumer startup)

Also updated P4 tests to match correct state machine (StreamCompleteEvent
keeps _message_registered=True, D1 handles reset on next RunStartedEvent).
Fixed P3 test assertion for same.

* fix: add sync() message count logging to diagnose duplication race

* diag: log message IDs in sync() response

* fix: remove persist_message_to_storage for user messages in prompt_async

Root cause of first-user-message duplication: the REST handler
(send_message_async) called persist_message_to_storage() to write
the user message to storage BEFORE the EventProcessor's
append_message_to_session (triggered via UserMessageInsertedEvent).

When the TUI calls sync() (GET /session/{id}/messages), it reads
from the same storage. If sync() runs after the REST handler's
write but before the SSE events are delivered, the TUI receives
the user message from BOTH sync() (storage) and SSE (PartUpdatedEvent).

Fix: remove persist_message_to_storage() call for user messages in
send_message_async. The EventProcessor handles persistence via
append_message_to_session, which runs atomically with SSE event
broadcast — no race window between storage write and SSE delivery.

* fix: prefer in-memory messages in sync() to eliminate part ID mismatch

get_messages_for_session() now returns in-memory MessageWithParts
(original part IDs matching SSE) when available, instead of DB-
reconstructed messages (new part IDs via chat_message_to_opencode).
This eliminates the root cause of first user message duplication:
TUI received parts from both sync() (DB, new IDs) and SSE (original
IDs), and couldn't deduplicate because IDs differed.

Added prefer_in_memory parameter (default True for sync/TUI).
Share/fork pass prefer_in_memory=False to use DB path (complete history).

168 tests pass.

* fix: strip user message parts from sync() to prevent TUI duplication

Root cause: TUI receives user message parts from TWO sources — sync()
REST API and SSE PartUpdatedEvent. The TUI has no part deduplication
mechanism (unlike CLI's replayedParts). Even with matching part IDs,
both sets of parts are rendered → duplicate text.

Fix: when prefer_in_memory=True (sync/TUI path), strip parts from
user messages in the response. SSE becomes the sole source of user
message parts. Assistant message parts are kept (TUI handles via
tracker.parts accumulation).

This is a single-source-of-truth approach: sync() provides message
metadata + assistant parts, SSE provides user message parts.
No duplication possible because each data type comes from one source.

168 tests pass.

* fix: strip user message parts from ALL sync() return paths

Previous fix only stripped parts from in-memory path. But sync() falls
through to DB path when in-memory is empty (first message race). DB
path via chat_message_to_opencode() returns parts with new IDs → TUI
gets parts from both DB (sync) and SSE → duplication.

Extracted _strip_user_parts() helper applied to all 3 return paths:
- In-memory fast-path
- DB (SessionPool) path
- Fallback in-memory path

Log evidence (session ses_0019f8a440fb6001):
  14:39:04.379 sync() returns 1 message (from DB, in-memory empty)
  14:39:05.079 append_message_to_session (700ms later, in-memory now has msg)

168 tests pass.

* fix: CI — ruff format/docstring + exempt subagent from user parts stripping

- ruff format: fix line length in _strip_user_parts
- ruff check: add prefer_in_memory to docstring Args section
- Integration: exempt subagent sessions from user parts stripping
  (subagents don't go through TUI sync, no duplication race)

168 unit + 647 integration tests pass. Ruff clean.

* fix: ruff lint + format for test files, rebase on main

Fix 11 ruff errors across 4 test files:
- SIM108: if-else → ternary
- BLE001: blind except → specific exception
- PT018: compound assert → separate asserts
- RUF059: unused variables prefixed with _
- F841: removed unused mock_append

Rebased on main (2 new commits: ty fix, logging noise reduction).
All pre-commit checks pass: ruff-format, ruff-fix, ty, ruff.

* fix(e2e): update tests for user parts stripping + finalization polling

4 E2E test fixes (all test issues, not real bugs):

1. test_prompt_async_user_message_renders_once: Check user message
   parts via SSE message.part.updated events instead of REST API
   (user parts now stripped from sync() response to prevent TUI
   duplication).

2. test_redflag_b3_queued_message_before_response: Comment out
   content check (user parts via REST). Primary ordering assertion
   preserved. User content now delivered via SSE only.

3. test_sync_message_sets_time_completed: Poll until time.completed
   is set, not just until 2 messages appear. Fixes race condition
   where assistant message exists before StreamCompleteEvent
   finalizes it.

4. test_sync_message_persists_to_storage: Same polling fix as #3.

* fix: rebase on main + ruff-fix auto-fixes

Rebased on main (4 new commits: CI fix, flatten_prompts refactor).
ruff-fix auto-fixed 9 files (unused imports, docstring formatting).
All pre-commit checks pass.

* fix: remove unused type: ignore comment (mypy)

main branch refactor made this type: ignore unnecessary.
Million-mo added a commit that referenced this pull request Aug 3, 2026
#1 tool_call_id: QuestionCapability._question already uses replace() to
propagate tool_name/tool_call_id/tool_input — verified with L2 test.
#2 telemetry: add @logfire.instrument to QuestionCapability._question;
background_task modules already instrumented.
#3 state cleanup: after_run() evicts _session_states and _ephemeral_states,
shuts down batcher and task manager.
#5 queued cancel: pending cancel path fires on_completed before
completion_event.set().
#6 flush exception: _flush catches broad Exception, marks delivered
regardless of success/failure.
#7 timeout message: CancelledError handler checks task.status ==
'timed_out' before choosing message.
#8 private API: guard pydantic_ai._agent_graph import with try/except
and helpful error message.
#9 L2 test: add test_question_tool_propagates_tool_call_id_to_agent_context.
#10 error contract: _build_definition raises ValueError with descriptive
message for missing name or non-dict input.
Nit: remove DEBUG_TASK_MGR prefix, type coro as Coroutine, fix test names.
Million-mo added a commit that referenced this pull request Aug 4, 2026
…Capability from xeno-agent (#346)

* feat(capabilities): add QuestionCapability with YAML schema override support

Move question tools into a proper AbstractCapability that accepts
args.schemas and args.enabled_tools, mirroring BackgroundTaskCapability.
Replaces the bare QuestionTools entry point so consumers can customize
LLM-facing parameter descriptions via YAML schema files without writing
their own capability wrapper.

* test(question): migrate question tool tests from xeno-agent

Move 51 question tool unit tests (40 for question_for_user + 11 for
ask_followup_question) from xeno-agent to agentpool. Tests now import
from agentpool_toolsets.builtin.question_tools instead of xeno_agent.

Two assertions adjusted to match agentpool's actual implementation:
- Error message regex: 'questionnaire' → 'questions' (agentpool naming)
- ask_followup_question metadata: dropped suggestion_attributes check
  (agentpool's _format_followup_response doesn't emit this field)

Also add RUF001 per-file ignore for CJK fullwidth punctuation in test data.

* feat(question): merge simple question tool into QuestionCapability

Add a 'question' tool to QuestionCapability that replicates the legacy
QuestionTool behavior (simple prompt + optional response_schema via MCP
Elicit). This unifies all user-interaction tools under one capability:
  - question_for_user: XML multi-question questionnaire
  - ask_followup_question: single question with <suggest> options
  - question: simplest single-question (replaces QuestionTool)

Mark QuestionToolConfig (tools: [{type: question}]) as deprecated,
directing users to capabilities: [{type: question}].

Add 3 new tests covering the question tool: default-enabled, enabled
alone, enabled via schemas.

* feat(capabilities): add BackgroundTaskCapability migrated from xeno-agent

Migrate the complete BackgroundTaskCapability implementation to agentpool:
- capability.py: full lifecycle management (task, background_output,
  background_cancel, steer_task tools)
- manager.py: BackgroundTaskManager with concurrent task execution,
  cleanup, and session isolation
- notification.py: NotificationBatcher for debounced completion notifications
- types.py: BackgroundTask, SessionTaskState dataclasses
- utils/tool_schema.py: YAML schema loading and LLM-facing schema override

Includes 277 tests (unit + integration + resource provider) covering
lifecycle, concurrency, error propagation, notification batching,
history isolation, and cancellation regression.

Config schema files (task.yaml, background_output.yaml,
background_cancel.yaml, steer_task.yaml) provide default LLM-facing
parameter descriptions.

Entry point 'background_task' registered in pyproject.toml.

* fix(ci): resolve ruff format, mypy, and flaky test failures

Three CI failures fixed:

1. ruff format: question.py ternary expression reformatted

2. mypy (16 errors → 0):
   - tool_schema.py: cast yaml.safe_load/json.loads results to
     OpenAIFunctionDefinition, construct TypedDict with explicit
     key-value pairs instead of ** unpacking
   - question.py: add ToolResult return annotations to tool wrappers
   - manager.py: re-read task_model.status into locals after await
     to prevent mypy narrowing from concurrent status changes
   - capability.py: type session_pool as SessionPool | None,
     fix delivered bool assignment from followup() str|None return,
     remove dead config.type == 'team' comparison (agents dict never
     contains team configs), rename shadowed task_model variable

3. Flaky tests (8 failures): replace fixed asyncio.sleep(0.8) with
   _wait_until_called() polling helper in 3 test files. The fixed
   sleep was too tight under CI load — debounce timers fire late
   when the event loop is busy with parallel workers.

* fix(ci): replace anyio TaskGroup with asyncio.ensure_future in NotificationBatcher

Root cause: loop.call_later callbacks run in a separate contextvars
context where anyio's sniffio async-library detection fails with
AsyncLibraryNotFoundError. This prevented _schedule_flush from
calling tg.start_soon, so _flush never executed and deliver_callback
was never invoked — all 25 batcher tests + 8 notification tests
failed in CI.

Fix: replace anyio.create_task_group/start_soon with
asyncio.ensure_future for scheduling flush coroutines. Keep
anyio.CancelScope and anyio.fail_after for timeout protection
(these don't require sniffio context). Track flush tasks in a
set[asyncio.Task] and cancel/await them in shutdown.

Also restore source_type team detection by checking config.type
via str() cast (mypy-safe for AnyAgentConfig union that doesn't
include team types at the type level, but mocks provide type='team'
at runtime).

* refactor(question): unify question tools into single question tool

Merge ask_followup_question, question_for_user, and question into one
unified question tool per reviewer feedback. The question_for_user
implementation (richest, supports multi-question XML with enum/multi/
input types) is retained as the canonical implementation, renamed to
question. ask_followup_question (legacy compat) and the simple question
tool are removed.

Changes:
- question_tools.py: remove ask_followup_question + _format_followup_response,
  rename question_for_user to question
- question.py: simplify QuestionCapability to expose only question
- Update all tests, docs, and tool name references

* fix(review): address opencode-agent review findings

#1 tool_call_id: QuestionCapability._question already uses replace() to
propagate tool_name/tool_call_id/tool_input — verified with L2 test.
#2 telemetry: add @logfire.instrument to QuestionCapability._question;
background_task modules already instrumented.
#3 state cleanup: after_run() evicts _session_states and _ephemeral_states,
shuts down batcher and task manager.
#5 queued cancel: pending cancel path fires on_completed before
completion_event.set().
#6 flush exception: _flush catches broad Exception, marks delivered
regardless of success/failure.
#7 timeout message: CancelledError handler checks task.status ==
'timed_out' before choosing message.
#8 private API: guard pydantic_ai._agent_graph import with try/except
and helpful error message.
#9 L2 test: add test_question_tool_propagates_tool_call_id_to_agent_context.
#10 error contract: _build_definition raises ValueError with descriptive
message for missing name or non-dict input.
Nit: remove DEBUG_TASK_MGR prefix, type coro as Coroutine, fix test names.

* chore: remove list_available_nodes tool (legacy, Leoyzen feedback #345)

- src/agentpool_toolsets/builtin/subagent_tools.py: remove
  list_available_nodes method + create_tool registration
- src/agentpool_config/toolsets.py: SubagentToolName Literal now
  only accepts 'task'; docstring updated
- tests/toolsets/test_tool_filtering.py: update assertions
- tests/toolsets/builtin/test_as_capability.py: update assertions
- tests/servers/acp_server/test_claude_acp_toolset_integration.py:
  update assertion
- tests/tools/test_runcontext.py: remove prompt referencing
  list_available_nodes (test was already xfail)
- docs/how-to/advanced/acp-integration.md: update docs
- docs/how-to/servers/mcp-server.md: update docs

The tool was legacy code; Leoyzen noted agents list is now
injected directly into system prompt.

* fix: ruff format toolsets.py (single-entry Literal syntax)
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