feat: unify tool interception to pydantic-ai capabilities - #106
Conversation
There was a problem hiding this comment.
Code Review
This pull request consolidates the team execution logic by merging the parallel Team and sequential TeamRun classes into a single BaseTeam class. It also unifies tool interception (confirmation, hooks, and injection) into a new _ToolInterceptCapability within the capability chain, simplifying wrap_tool and removing the obsolete inject_cancelled_tool_results helper. The review feedback highlights several critical improvements: wrapping child session closure in a try-except block to ensure all sessions close gracefully, adding a None check on self.member_prompt_templates to prevent a runtime AttributeError, and implementing precise tool execution duration tracking in hook_manager.py instead of hardcoding it to 0.0.
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.
…uration tracking - base_team.py: wrap child session close in try-except to prevent cascade failures when closing multiple scoped team sessions - base_team.py: guard member_prompt_templates against None before calling .get() for defensive type safety - hook_manager.py: store tool start time in wrap_tool_execute via _tool_start_times dict on agent_ctx, compute real duration_ms in after_tool_execute instead of hardcoded 0.0 - Replace hasattr with getattr + explicit None check per type-safety rules Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
…uration tracking - base_team.py: wrap child session close in try-except to prevent cascade failures when closing multiple scoped team sessions - base_team.py: guard member_prompt_templates against None before calling .get() for defensive type safety - hook_manager.py: store tool start time in wrap_tool_execute via _tool_start_times dict on agent_ctx, compute real duration_ms in after_tool_execute instead of hardcoded 0.0 - Replace hasattr with getattr + explicit None check per type-safety rules Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
26f5123 to
63462ed
Compare
Replace dual-path tool interception (wrap_tool legacy + capability chain) with a unified _ToolInterceptCapability that handles confirmation, hooks, error handling, and injection uniformly across all tool sources (direct, MCP, ACP). Changes: - Add _ToolInterceptCapability in hook_manager.py with get_wrapper_toolset (ApprovalRequiredToolset), prepare_tools, wrap_tool_execute, before_tool_execute (ModelRetry on deny), after_tool_execute (modified_output/additional_context/injection consumption) - as_capability() returns CombinedCapability[_ToolInterceptCapability, hooks_cap] with hooks_cap stripped of tool callbacks (Decision 2) - Remove if not self.hooks guard in get_agentlet() — always register - Simplify wrap_tool() to only AgentContext injection + deferred execution - Remove _execute_with_hooks, handle_confirmation, _handle_confirmation_result - Remove redundant mode == 'never' auto-approve in approval_bridge.py - Update 2 tests for new never-mode behavior (bridge routes to provider) - Update 3 ACP snapshots (ToolResult now converts to ToolReturn) Spikes verified: - ModelRetry from before_tool_execute is caught by pydantic-ai (Spike 0.1) - Nested ApprovalRequiredToolset short-circuits via ctx.tool_call_approved (Spike 0.2) - Hooks._registry supports stripping individual callbacks (Spike 0.3) Closes openspec change: unify-tool-interception-to-pydantic-ai-capabilities
…5.1-5.13) - 10 unit tests: get_wrapper_toolset (always/never/per_tool), wrap_tool_execute (error handling + pass-through), before_tool_execute (modified_input + deny via ModelRetry), after_tool_execute (modified_output, additional_context, injection consumption) - 3 integration tests: hooks fire for MCP tools, confirmation works for MCP tools when mode=always, no double-firing when old AgentHooks is active - All 43/43 tasks complete for unify-tool-interception-to-pydantic-ai-capabilities Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
- hook_manager.py: store tool start time in wrap_tool_execute via _tool_start_times dict on agent_ctx, compute real duration_ms in after_tool_execute instead of hardcoded 0.0 - Replace hasattr with getattr + explicit None check per type-safety rules - base_team.py review comments (try-except, None guard) are auto-resolved: the referenced code (_close_scoped_team_nodes, _resolve_member_prompt) does not exist in origin/refactor/thin-wrapper after rebase Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
63462ed to
0b6d8f1
Compare
Documents the audit of 4 existing hooks (pre_run, post_run, pre_tool_use, post_tool_use) against pydantic-ai Capability hooks. Results: - pre_run/post_run: kept (run-level lifecycle, no Capability equivalent) - pre_tool_use/post_tool_use: migrated to _ToolInterceptCapability (PR #106)
…74) (#93) * refactor(orchestrator): split core.py into event_bus, session_controller, session_pool Phase 1 of thin-wrapper refactor (#74): Split the 3035-line orchestrator/core.py into three focused modules: - event_bus.py (595 lines): EventBus, EventEnvelope, drain_and_merge, and all merge helper functions - session_controller.py (1382 lines): SessionController, SessionState, exceptions, SessionLifecyclePolicy - session_pool.py (1145 lines): SessionPool core.py reduced to thin re-exports for backward compatibility. All 460 orchestrator tests pass, mypy clean, ruff clean. Also includes OpenSpec change artifacts (proposal, design, specs, tasks) for the full 8-phase thin-wrapper refactor plan. * test(agents): verify node-level Capability hooks on standalone run path (#95) * fix(orchestrator): address 8 Gemini Code Assist review comments on PR #93 - start_cleanup_task: start _start_cleanup_loop + strong task refs in _background_tasks set - get_or_create_session_agent: validate session_id non-empty before proceeding - create_team_from_config: use stateless cfg.get_agent() instead of session-bound agent (fixes MCP subprocess leak) - _close_session_unlocked: wrap child session close in try-except (cascade resilience) - close_session: same try-except for second child close loop - event_bus _drain_dead_streams: catch all exceptions, not just anyio-specific ones - event_bus close_session: same broad exception handling for send_stream aclose - session_pool close_session: wrap in try-finally so EventBus + cache cleanup always runs 460 orchestrator tests pass, ruff + mypy clean. * chore: limit pre-commit pytest to orchestrator subset (full suite in CI) * refactor(agents): Phase 2 — simplify run_stream producer/consumer to direct delegation Remove the redundant producer/consumer pattern from BaseAgent.run_stream() Path B. Previously, _run_stream_once() ran in an asyncio.ensure_future producer task that published events to EventBus, then the consumer drained them via drain_and_merge(). Now run_stream() directly iterates _run_stream_once() with inline event handler dispatch. Key finding: NativeTurn.execute() already calls agent_run.next(node) (line 206 of turn.py), so pdai Capability hooks were already firing on all paths. The producer/consumer was redundant indirection, not a hook-breaking bug. Changes: - BaseAgent.run_stream(): remove _producer task + drain_and_merge consumer, replace with direct async for on _run_stream_once() - Event handler dispatch moved inline (was in consumer loop) - EventBus cleanup preserved (close_session on local bus) - New test: test_capability_hooks_standalone.py verifies wrap_run hook fires on standalone run_stream() path Verification: 837 tests pass (agents + orchestrator), ruff clean. * fix(orchestrator): restore producer/consumer pattern + fix ruff lint Phase 2's direct delegation removed EventBus subscription, losing events that bypass _stream_events() (ToolCallProgressEvent, SpawnSessionStart). Restore producer/consumer with asyncio.Queue-compatible drain_and_merge. Also fix: - F401: remove unused EventBus import in _stream_events() - I001: fix import sorting in test_capability_hooks_standalone.py Fixes CI failures: - test_workers_child_session_persisted_with_correct_parent (SpawnSessionStart) - test_progress_handler_with_agent_non_streaming/streaming (progress events) - test_agent_stream_progress_events (ToolCallProgressEvent) - Lint (ruff check) failures * test(agents): verify node-level Capability hooks on standalone run path Address Gemini Code Assist review on PR #95: the previous test only verified wrap_run (a run-level hook triggered by agentlet.iter()), which would fire even on the legacy path. Phase 2's core invariant is that agent_run.next(node) is called on the standalone path, triggering node-level hooks. Expand HookTrackerCapability to implement and track: - wrap_node_run - before_model_request - after_node_run (with correct signature including node param) Rename test to test_capability_hooks_fire_on_standalone_run to reflect that it verifies all four hooks, not just wrap_run. Verification: 1 test passes, ruff + mypy clean. --------- Co-authored-by: Test <test@test.com> * refactor(orchestrator): Phase 3 — migrate EventBus to asyncio.Queue (#96) * refactor(orchestrator): Phase 3 — migrate EventBus from anyio streams to asyncio.Queue Replace anyio memory object streams with asyncio.Queue throughout the EventBus and all consumers. Introduces configurable overflow policies (drop_oldest, drop_newest, drop_subscriber) replacing the previous hybrid timeout→drop strategy. Production changes: - EventBus: anyio.Lock → asyncio.Lock, memory streams → asyncio.Queue - EventBus: new _enqueue() method with overflow policy dispatch - EventBus: drain_and_merge() now drains asyncio.Queue (QueueShutDown/QueueEmpty) - session_pool.py: stream.receive()→queue.get(), receive_nowait()→get_nowait() - session_pool.py: anyio.EndOfStream→asyncio.QueueShutDown, WouldBlock→QueueEmpty - mixins.py: _consumer_streams type annotation anyio.abc→asyncio.Queue Test changes: - All tests using 'async for event in queue' replaced with queue.get() loop - gen.shutdown()→gen.aclose() (async generators have no shutdown()) - anyio.fail_after() used instead of asyncio.timeout() (anyio pytest plugin) - _stream_empty via statistics()→queue.empty() - test_event_bus_backpressure.py: rewritten for asyncio.Queue API - test_replay_buffer_turn_isolation.py: rewritten for queue.get() - test_sessionpool_reasoning_redflag.py: _drain_queue() helper Verification: - 460 orchestrator tests pass (0 failures) - ruff: 0 errors * fix(orchestrator): complete asyncio.Queue migration + adopt Gemini review on PR #96 Fixes all CI failures from Phase 3 (mypy, unit, integration, core tests): Production fixes: - acp_agent.py: migrate bus_stream from anyio stream API (.receive()/ .receive_nowait()) to asyncio.Queue API (.get()/.get_nowait()), add QueueShutDown handling, fix type annotation, remove unused import - event_bus.py: fix mypy comparison-overlap error in _enqueue overflow policy dispatch (check 'block' before membership test) - mixins.py: fix consumer task lifecycle (await task exit before cleanup, suppress exceptions during unsubscribe), add contextlib import - message_routes.py + session_pool_integration.py: wrap raw queue iteration in drain_and_merge() (Queue has no async-for protocol) Test fixes (Gemini Code Assist review): - test_performance.py + test_resume_session.py: replace destructive _stream_empty (get_nowait consumes items) with non-destructive queue.empty() - test_event_bus.py: test_close_session_signals_end_of_stream now drains queue in a loop until QueueShutDown (was only calling get_nowait once) - test_event_bus_backpressure.py: rename misleading test to test_backpressure_retains_subscriber_with_drop_oldest (default policy is drop_oldest, subscriber is NOT dropped) - All consumer tests: migrate from anyio stream API to asyncio.Queue API (get/get_nowait/QueueShutDown/QueueEmpty) - mock_stream.py: update helper to use asyncio.Queue instead of anyio streams Verification: 1244 tests pass, mypy clean, ruff clean. --------- Co-authored-by: Test <test@test.com> * feat(config): Phase 4 — build graph_translation.py + extend GraphStepConfig - Create src/agentpool_config/graph_translation.py with translate_team_to_graph, translate_teams_to_graphs, translate_connections_to_edges, translate_config_to_graph, build_steps_from_agents - Extend GraphStepConfig with team fields: shared_prompt, prompt_template, member_timeout, member_retry_attempts, member_retry_delay - Export all new types and functions from agentpool_config.__init__ - 22 unit tests covering all translation paths * feat(manifest): Phase 4 Part 2 — auto-translate teams: to graph: in manifest - Add explicit graph: GraphConfig | None field to AgentsManifest - Add _auto_translate_teams_to_graph model validator that runs after _populate_node_names: when graph is None, translates teams: and connections: into a unified GraphConfig - teams: mode=sequential produces chained steps (start->s1->s2->end) - teams: mode=parallel produces Fork+Join (start->[all], [all]->end) - Existing graph: section takes precedence (no translation) - 482 orchestrator+config tests pass, ruff clean * deprecate(teams): mark TeamConfig.get_team() as deprecated Add DeprecationWarning directing users to translate_team_to_graph() from agentpool_config.graph_translation. Full Team/TeamRun removal requires restoring pool-level graph execution — deferred to a later phase. * fix(config): address Gemini review on PR #97 — connection translation + openspec sync - Fix KeyError: skip FileConnectionConfig/CallableConnectionConfig in translate_connections_to_edges (only NodeConnectionConfig has 'name') - Fix implicit truthiness on teams dict (use 'is not None' per review) - Simplify downstream conditional (remove redundant 'and teams is not None') - Add 6 unit tests: node→edge, file skip, callable skip, mixed, full config translation, empty teams dict - Update openspec tasks.md: mark Phase 4 tasks 4.1-4.9 as completed - Update openspec spec.md: add 'Agent connections translated to graph edges' requirement with skip-scenarios for file/callable connections * feat(tools): Phase 5 — ToolsetFactory protocol replacing ResourceProvider - Define ToolsetFactory as runtime_checkable Protocol with create_capability() -> AbstractCapability | None - Implement StaticToolsetFactory: wraps pre-configured Tool list, produces FunctionToolset/ApprovalRequiredToolset/CombinedToolset - Implement AdapterToolsetFactory: wraps existing ResourceProvider for incremental migration (delegates to provider.as_capability()) - Protocol is structural (duck-typed) matching pdai's own Toolset style * fix(tools): address Gemini review on PR #98 — extract tool wrapping util, remove dead code - Extract _wrap_for_pydantic_ai into shared tool_wrapping.py module - StaticToolsetFactory now calls wrap_tool_for_pydantic_ai directly, removing tight coupling to deprecated ResourceProvider (comment #1) - ResourceProvider._wrap_for_pydantic_ai delegates to shared util for backwards compatibility - Remove redundant 'if not toolsets:' dead code (comment #2) - Fix return type: AbstractToolset[Any] | None (was AbstractCapability) - Clean up unused imports (inspect, ModelRetry) from base.py - Rebase onto latest refactor/thin-wrapper (includes PR #97) * chore(ci): Phase 7 — add import-linter for boundary enforcement Add import-linter as dev dependency with 3 forbidden contracts: 1. Server must not import from CLI/commands 2. Config must not import from core 3. ACP package must not import from server Current status: 3 contracts broken (systemic circular deps). Violations are pre-existing architectural issues requiring gradual refactoring across hundreds of files. Contracts are documented as known violations; fixes will be incremental. * fix(ci): address Gemini review on PR #99 — add ignore_imports + allow_indirect_imports - Add ignore_imports for all 3 forbidden contracts listing pre-existing direct violations (8 server→cli/commands, 72 config→core, 0 acp→server) - Set allow_indirect_imports = true for all contracts to keep CI green while preventing NEW direct boundary violations - Comments document the migration path (remove entries as imports are fixed) - Update openspec tasks.md: mark 7.1-7.3 done, update 7.3/7.10 descriptions - Mark 5.1 and 5.5 done (PR #98 merged ToolsetFactory protocol + adapters) * feat(capabilities): Phase 6 — 6 pdai Capability implementations - LoopDetectionCapability: prevents infinite delegation loops via wrap_node_run depth tracking, raises LoopDetectionError at max_depth - TokenBudgetCapability: enforces token budget per run via wrap_model_request, raises TokenBudgetExceededError - ToolOutputBudgetCapability: truncates tool output via wrap_tool_execute when exceeding max_output_chars - DynamicContextCapability: compacts conversation history via before_model_request when approaching context limit - SkillActivationCapability: dynamic per-turn skill injection via before_model_request, supersedes SkillBridgeCapability - MemoryCapability: persistent key-value memory across turns via after_node_run (persist) + before_model_request (inject) All capabilities implement AbstractCapability with for_run() returning fresh per-run copies. Wiring into get_agentlet() is blocked on Phase 2 (Run Stream Unification) — hooks only fire via RunExecutor.next(node). 19 unit tests pass, ruff clean. * chore: move mypy and pytest from pre-commit to pre-push stage Move heavy hooks (mypy full scan ~16s, pytest orchestrator suite ~30s) from pre-commit to pre-push to keep commits fast (~2-5s). Pre-push runs mypy + unit-marked tests as a safety net; full suite still runs in CI. * chore: ignore .worktrees directory * fix(capabilities): address Gemini review on PR #100 Critical fixes: - LoopDetectionCapability: use contextvars.ContextVar for depth tracking (instance _depth was reset to 0 after each node_run, and for_run() copies lost depth across agent delegation boundaries) - MemoryCapability: inject into SystemPromptPart.content (ModelMessage has no system_prompt attribute; getattr always returned None) - SkillActivationCapability: same SystemPromptPart fix High fix: - MemoryCapability for_run(): share _store dict reference instead of shallow copy (memories were discarded after each run) Medium fixes: - DynamicContextCapability: log warning when compaction triggered but no fn - ToolOutputBudgetCapability: reuse self._truncate() in case str() block - Add unit tests for DynamicContext, SkillActivation, Memory, and _inject_into_system_prompt helper CI fixes: - Fix mypy: move imports from pydantic_ai.agent/result to pydantic_ai.capabilities (AgentNode, NodeResult, WrapNodeRunHandler, etc.) - Fix mypy: for_run() must be async (parent declares async) - Fix mypy: handler signatures (positional args, not keyword) - Fix ruff format Verified: 54 tests pass, ruff clean, mypy clean (7 files, 0 issues) * feat(scripts): Phase 8 — automated rename script agentpool → agentwolf Python script that performs one-shot mechanical rename: - Renames 10 src/ directories (agentpool → agentwolf, agentpool_* → agentwolf_*) - Replaces all Python imports (1215 files in dry run) - Updates pyproject.toml package names and entry points - Updates YAML configs, Markdown docs, JSON schemas - Excludes openspec/changes/ and .omo/ (historical artifacts) Usage: python scripts/rename_to_agentwolf.py [--dry-run] Pre-requisite: all Phase 1-7 PRs merged into refactor/thin-wrapper. The rename must be executed as a single atomic commit after merge. * fix(scripts): address Gemini review on PR #101 - EXCLUDE_DIRS: use Path.is_relative_to() instead of string 'in parts' (multi-segment paths like 'openspec/changes' were never matched) - rename_directories: refuse shutil.move if destination already exists (prevents nesting src/agentwolf/agentpool on repeated runs) - Remove redundant update_pyproject() — pyproject.toml already handled by replace_references() via *.toml pattern - check_entry_points: warn on missing files instead of silent skip - Pre-compile EXCLUDE_DIRS as list[Path] (avoid per-iteration allocation) - Update docstring steps to match new structure * docs(openspec): sync tasks.md + create 5 follow-up changes for incomplete phases tasks.md fixes: - Mark Phase 3 tasks 3.1-3.14 as [x] (PR #96 merged, was never updated) - Add task 8.0 for rename script (PR #101 merged) 5 follow-up openspec changes created for remaining work: - followup-team-removal: Phase 4 — remove Team/TeamRun, migrate 50 callers - followup-toolset-factory-migration: Phase 5 — create 3 factories, migrate 70 callers, remove ResourceProvider - followup-capability-wiring: Phase 6 — YAML config support, Agent class wiring, hook audit - followup-server-boundary-fixes: Phase 7 — fix 80 import-linter violations, add CI - followup-agentwolf-rename-execution: Phase 8 — execute rename script, verify Status: 64/123 tasks done (52%). Phases 1-3 100%, Phase 4 50%, Phase 5 12%, Phase 6 76%, Phase 7 27%, Phase 8 4%. * fix(openspec): add scenarios to all 5 follow-up change specs All 5 follow-up openspec changes failed validation because spec.md requirements lacked required #### Scenario: blocks. Added scenarios to all 25 requirements across 5 spec files. Also fixed: SkillBridgeCapability requirement now contains SHALL (was flagged for missing SHALL/MUST keyword). All 5 changes now pass 'openspec validate'. * refactor(openspec): merge 5 follow-up changes into single followup-thin-wrapper-refactor Consolidates 5 separate follow-up openspec changes into one unified change with 5 spec modules and 63 tasks: - specs/team-removal: Phase 4 (11 tasks) - specs/toolset-migration: Phase 5 (15 tasks) - specs/capability-wiring: Phase 6 (15 tasks) - specs/boundary-fixes: Phase 7 (11 tasks) - specs/rename-execution: Phase 8 (11 tasks) Removed: followup-team-removal, followup-toolset-factory-migration, followup-capability-wiring, followup-server-boundary-fixes, followup-agentwolf-rename-execution Validated: openspec validate followup-thin-wrapper-refactor passes. * feat: unify tool interception to pydantic-ai capabilities Replace dual-path tool interception (wrap_tool legacy + capability chain) with a unified _ToolInterceptCapability that handles confirmation, hooks, error handling, and injection uniformly across all tool sources (direct, MCP, ACP). Changes: - Add _ToolInterceptCapability in hook_manager.py with get_wrapper_toolset (ApprovalRequiredToolset), prepare_tools, wrap_tool_execute, before_tool_execute (ModelRetry on deny), after_tool_execute (modified_output/additional_context/injection consumption) - as_capability() returns CombinedCapability[_ToolInterceptCapability, hooks_cap] with hooks_cap stripped of tool callbacks (Decision 2) - Remove if not self.hooks guard in get_agentlet() — always register - Simplify wrap_tool() to only AgentContext injection + deferred execution - Remove _execute_with_hooks, handle_confirmation, _handle_confirmation_result - Remove redundant mode == 'never' auto-approve in approval_bridge.py - Update 2 tests for new never-mode behavior (bridge routes to provider) - Update 3 ACP snapshots (ToolResult now converts to ToolReturn) Spikes verified: - ModelRetry from before_tool_execute is caught by pydantic-ai (Spike 0.1) - Nested ApprovalRequiredToolset short-circuits via ctx.tool_call_approved (Spike 0.2) - Hooks._registry supports stripping individual callbacks (Spike 0.3) Closes openspec change: unify-tool-interception-to-pydantic-ai-capabilities * test: add _ToolInterceptCapability unit and integration tests (tasks 5.1-5.13) - 10 unit tests: get_wrapper_toolset (always/never/per_tool), wrap_tool_execute (error handling + pass-through), before_tool_execute (modified_input + deny via ModelRetry), after_tool_execute (modified_output, additional_context, injection consumption) - 3 integration tests: hooks fire for MCP tools, confirmation works for MCP tools when mode=always, no double-firing when old AgentHooks is active - All 43/43 tasks complete for unify-tool-interception-to-pydantic-ai-capabilities Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> * fix: address PR #106 review comments — duration tracking in hook_manager - hook_manager.py: store tool start time in wrap_tool_execute via _tool_start_times dict on agent_ctx, compute real duration_ms in after_tool_execute instead of hardcoded 0.0 - Replace hasattr with getattr + explicit None check per type-safety rules - base_team.py review comments (try-except, None guard) are auto-resolved: the referenced code (_close_scoped_team_nodes, _resolve_member_prompt) does not exist in origin/refactor/thin-wrapper after rebase Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> * refactor: Phase 4 — merge Team/TeamRun into BaseTeam with mode parameter Remove Team and TeamRun classes, consolidating their execution logic into a single concrete BaseTeam that dispatches based on mode ('parallel' or 'sequential'). Changes: - BaseTeam is now concrete with mode parameter (was abstract) - Team.execute() → BaseTeam._execute_parallel() (Fork+Join graph) - TeamRun.execute() → BaseTeam._execute_sequential() (chained graph) - execute_iter(), run(), run_iter(), run_stream() all dispatch by mode - ExtendedTeamTalk and _TeamRunGraphState moved to graph_team.py - __or__/__and__ operators return BaseTeam instead of Team/TeamRun - TeamConfig.get_team() creates BaseTeam instead of Team/TeamRun - All 17 caller files updated to use BaseTeam - team.py and teamrun.py deleted (1081 lines removed) - tasks.md updated with Phase 4 completion status Verification: 4218 tests pass, mypy clean, ruff clean * feat: Phase 5 — create 3 ToolsetFactory classes, add deprecation warnings Add MCPToolsetFactory, LocalSkillToolsetFactory, PoolToolsetFactory to tools/factory.py, each wrapping their respective ResourceProvider and implementing the ToolsetFactory protocol. Add DeprecationWarning to ResourceProvider.__init__, CodeModeResourceProvider.__init__, and RemoteCodeModeResourceProvider.__init__ pointing users to ToolsetFactory-based approach. Suppress internal DeprecationWarning cascades in ToolManager and MCPManager since their internal ResourceProvider usage is an implementation detail of deprecated code. Verification: 4218 tests pass, mypy clean, ruff clean * feat: Phase 6 — complete capability wiring, hook audit docs, reconciliation - Add hook migration audit to hook_manager.py module docstring (pre_run, post_run, pre_tool_use, post_tool_use status) - Add 6 typed capability config models in capabilities.py (LoopDetection, TokenBudget, ToolOutputBudget, DynamicContext, SkillActivation, Memory) as discriminated union with backward compat fallback to GenericCapabilityConfig - Improve handle_capabilities validator with typed validation - Add capabilities parameter to Agent.__init__() - Add reconciliation docstrings to SkillActivationCapability and ToolOutputBudgetCapability - Add capability hooks verification tests for graph run paths - Update tasks.md with Phase 6 completion status Verification: 446 agent/capability tests pass, mypy clean, ruff clean * ci: Phase 7 — add lint-imports to CI pipeline, fix new import violation * docs: mark Phase 8 as blocked — deferred by user, requires explicit confirmation * feat(mcp): add _toolset_cache to MCPManager for MCPToolset connection reuse as_capability() now caches MCPToolset instances by client_id, avoiding duplicate MCP connections for the same server config. disconnect_all() closes cached toolsets via __aexit__ (with ValueError guard for unentered toolsets) before clearing the cache. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> * test(mcp): update tests for _toolset_cache caching behavior Update 9 tests across 5 files to accept cached MCPToolset instances. Add regression test verifying cross-task shared toolsets don't raise CancelScope errors. Add tests for distinct client_ids and cache cleanup. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> * chore(openspec): archive migrate-to-mcptoolset change All implementation tasks complete (40/41). Task 8.8 (manual QA) deferred. Delta spec not synced to main specs — thin-wrapper refactor will redefine MCP integration specs. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> * fix(test): replace isinstance with Annotated Union-safe helper in capabilities_yaml tests CapabilityConfig is an Annotated[Union[...]] type which cannot be used with isinstance(). Replace with a helper that checks against concrete config types. Adapted from commit 0ea40eb (lost in force-push). Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> * refactor: Phase 4 + Phase 7 (server→cli) — remove TeamConfig.get_team(), move NodeCommand to core, fix 8 server→cli import violations Phase 4: - Remove TeamConfig.get_team() from agentpool_config/teams.py (config→core violation) - Move team creation logic to _build_team_from_config() in session_pool.py (core layer) - Update resource_providers/pool.py to use _build_team_from_config() - Fix stale docstrings referencing removed Team/TeamRun classes - Remove 3 ignore_imports entries for agentpool_config.teams Phase 7 (server→cli): - Move NodeCommand and AgentCommand from agentpool_commands.base to agentpool.commands.base - agentpool_commands.base now re-exports for backward compatibility - Update 7 ACP server command files to import from agentpool.commands.base - Use importlib for runtime agent_cli import in debug_commands.py - Remove all 8 server→cli ignore_imports entries * feat: Phase 6 — wire capabilities: YAML config section into Agent class - Add capabilities field to BaseAgentConfig (list[CapabilityConfig]) - Wire capabilities from config into Agent.from_config() via build_capability() - Add _build_capability_from_config() helper with late import to avoid config→core static dependency - Supports both built-in capabilities (loop_detection, token_budget, etc.) and generic import-path capabilities * docs: Phase 6 — hook audit document for Capability overlap analysis Documents the audit of 4 existing hooks (pre_run, post_run, pre_tool_use, post_tool_use) against pydantic-ai Capability hooks. Results: - pre_run/post_run: kept (run-level lifecycle, no Capability equivalent) - pre_tool_use/post_tool_use: migrated to _ToolInterceptCapability (PR #106) * docs: update tasks.md — mark Phase 4/5/6/7 completed items Phase 4: 4.11-4.13, 4.16-4.18 completed (Team/TeamRun removed, get_team() removed) Phase 5: 5.2-5.4, 5.10 completed (3 factories exist, deprecation warnings added) Phase 6: 6.14-6.16 completed (hook audit, YAML config, Agent wiring) Phase 7: 7.4-7.7 completed (server→cli violations fixed, 3 config→core fixed) * docs: update tasks.md — mark Phase 4/6/7 completed items, defer Phase 7 config→core violations to #114 Phase 4 (Team Cleanup): - 4.10: Translator tested against all teams: YAML configs (28 tests pass) - 4.14: All TeamRun callers already migrated to BaseTeam (zero direct imports remain) - 4.15: _TeamGraphState/_TeamRunGraphState are active internal implementation, not legacy Phase 6 (Capabilities): - 6.17: tests/agents/ + tests/capabilities/ all pass (446 tests, 0 failures) Phase 7 (Server Boundaries): - 7.8/7.10: 71 config→core violations deferred to #114 (import-linter detects TYPE_CHECKING and lazy imports; needs architectural decision on config↔runtime separation — 42 TYPE_CHECKING, 1 function-level, 5 module-level) - 7.11: Full test suite passes (CI green) Followup tasks.md updated to reflect actual completion status. * fix: resolve merge conflict — apply PR #111 last_active_at update to session_controller.py PR #111 (4c38b2b) updated SessionController.receive_request() to refresh last_active_at on every incoming message, preventing premature TTL cleanup. After Phase 1 core.py split, SessionController moved to session_controller.py. This commit applies the same fix to the new file location. * chore: remove 2,318 lines of dead test code - Delete test_concurrent_safety.py (entire file unconditionally skipped) - Delete 3 collect_ignore files (phase2_native_queue, steer_followup_edge_cases, steer_followup_integration) - Delete test_input_provider.py (6 xfail tests for unimplemented RFC-0015) - Delete test_message_timeout.py (pre-SessionPool code path) - Delete src/acp_v2/ and tests/servers/acp_server/v2/ ghost directories (.pyc only) - Remove 8 run/turn separation skip tests across 5 files - Remove 4 tests referencing removed APIs (pool.get_agents, SubagentTools.task) - Clean up collect_ignore entries from conftest.py - Fix unused imports flagged by ruff --------- Co-authored-by: Test <test@test.com> Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Summary
Replace dual-path tool interception (wrap_tool legacy + capability chain) with a unified
_ToolInterceptCapabilitythat handles confirmation, hooks, error handling, and injection uniformly across all tool sources (direct, MCP, ACP).Problem
AgentPool had two parallel tool interception mechanisms:
wrap_tool()→ confirmation + hooks + injection → direct tools onlyThis meant only direct tools got confirmation and hooks; MCP and ACP tools bypassed them entirely. Additionally,
AgentHooks._wrap_after_tool_executediscardedmodified_outputandadditional_contextfrom hook results (bug).Solution
New
_ToolInterceptCapability(hook_manager.py)Extends
AbstractCapabilitywith:get_wrapper_toolset(): wraps toolset withApprovalRequiredToolsetbased ontool_confirmation_mode(always/never/per_tool)before_tool_execute(): runs pre-tool hooks, denies viaModelRetry(notRuntimeError)after_tool_execute(): runs post-tool hooks, appliesmodified_output/additional_context, consumesPromptInjectionManagerinjectionswrap_tool_execute(): catches exceptions, returns annotatedToolReturnprepare_tools(): placeholder for future schema modificationas_capability()returnsCombinedCapability[_ToolInterceptCapability(), hooks_cap]wherehooks_capis stripped of all tool callbacks (before_tool_execute,after_tool_execute) and lifecycle callbacks (before_run,after_run) to prevent double-firing.Simplified
wrap_tool()Removed:
handle_confirmation(),_execute_with_hooks(),_handle_confirmation_result()Retained: AgentContext injection, deferred execution support, ToolResult→ToolReturn conversion
Approval bridge cleanup
Removed redundant
mode == "never"auto-approve check — in never mode,ApprovalRequiredToolsetis not applied, so deferred requests never reach the bridge.Spikes Verified
ModelRetryfrombefore_tool_executeIS caught by pydantic-ai agent loop and retriedctx.tool_call_approvedidempotency flag prevents double-deferral_registryis a dict; settingkey = []makes method a no-opTest Results
OpenSpec
Closes change:
unify-tool-interception-to-pydantic-ai-capabilities