diff --git a/.gitignore b/.gitignore index 192fbae8e..58ab4f098 100644 --- a/.gitignore +++ b/.gitignore @@ -62,3 +62,6 @@ test_configs/ # Prometheus / OpenCode workspace files .omo/ + +# Codegraph local index +.codegraph/ diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 000000000..14eefcb99 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,25 @@ +# Changelog + +## [Unreleased] - PydanticAI Thinning Refactor + +### Added +- `HooksCapabilityAdapter` (`src/agentpool/agents/native_agent/hooks_capability_adapter.py`) — new bridge that builds `pydantic_ai.capabilities.Hooks` from AgentPool hook callables with priority combining (deny > ask > allow), matcher filtering, and return type normalization +- `from_agent_hooks()` factory method on `HooksCapabilityAdapter` — extracts `fn`, `matcher`, `input_match` from existing `CallableHook`/`CommandHook`/`PromptHook` instances for transparent migration +- 151 regression tests covering hook combination, ProcessHistoryAdapter, PromptInjectionManager, Tool conversion, and event subclass behavior + +### Changed +- `NativeAgentHookManager.as_capability()` now uses `HooksCapabilityAdapter.from_agent_hooks()` instead of `AgentHooks.as_capability()` — injection consumption wrapping preserved +- `AGENTS.md` updated with new architecture conventions (Hooks delegation, ProcessHistory direct usage, event passthrough, ToolKind deprecation) + +### Deprecated +- `AgentHooks.as_capability()` — emits `DeprecationWarning`. Use `HooksCapabilityAdapter.from_agent_hooks()` instead +- `Hook` ABC and subclass hierarchy (`CallableHook`, `CommandHook`, `PromptHook`) — remain functional for YAML config instantiation but no longer the primary integration path +- `ProcessHistoryAdapter` — `get_agentlet()` already uses `pydantic_ai.capabilities.ProcessHistory` directly +- `ToolKind` literal type — use string-based tool name patterns instead +- `ToolResult.structured_content` — use PydanticAI's native `ToolReturn` structured return mechanism +- `PartStartEvent`/`PartDeltaEvent` PydanticAI subclassing — `session_id` should be accessed via `AgentContext` or `RunContext.deps` + +### Notes +- No behavioral changes — all 734 existing unit tests pass at the same rate as baseline +- 39 pre-existing test failures and 14 errors on `develop/agentic` branch remain unchanged (not caused by this refactor) +- ACP agent path fully preserved — `PromptInjectionManager.queue()`/`pop_queued()`/`flush_pending_to_queue()` remain for non-native agents diff --git a/docs/migration/pydanticai-thinning.md b/docs/migration/pydanticai-thinning.md new file mode 100644 index 000000000..f93462bae --- /dev/null +++ b/docs/migration/pydanticai-thinning.md @@ -0,0 +1,109 @@ +# PydanticAI Thinning Refactor — Migration Guide + +This guide covers the deprecations introduced by the PydanticAI thinning refactor and how to migrate existing code. + +## Overview + +The refactor makes AgentPool "thinner" at the agent-engine layer by delegating to PydanticAI's native capabilities. Old APIs remain functional with `DeprecationWarning` — no immediate breakage. + +## 1. Hooks: `AgentHooks.as_capability()` → `HooksCapabilityAdapter` + +**Before:** +```python +from agentpool.hooks import AgentHooks, CallableHook + +hooks = AgentHooks(pre_run=[CallableHook(event="pre_run", fn=my_hook)]) +capability = hooks.as_capability() # DeprecationWarning +``` + +**After:** +```python +from agentpool.agents.native_agent.hooks_capability_adapter import HooksCapabilityAdapter + +# Option A: From existing AgentHooks (transparent migration) +adapter = HooksCapabilityAdapter.from_agent_hooks(hooks) +capability = adapter.build() + +# Option B: Direct construction (preferred for new code) +adapter = HooksCapabilityAdapter( + before_run=[my_hook], +) +capability = adapter.build() +``` + +**YAML config**: No changes needed. `CallableHook`, `CommandHook`, `PromptHook` remain functional — the adapter extracts their `fn`, `matcher`, and `input_match` transparently. + +## 2. ProcessHistoryAdapter + +**Status**: `get_agentlet()` already uses `pydantic_ai.capabilities.ProcessHistory` directly (line 869 of `agent.py`). `ProcessHistoryAdapter` is deprecated but remains for manual usage. + +**Before:** +```python +from agentpool.agents.native_agent.process_history_capability import ProcessHistoryAdapter +caps = ProcessHistoryAdapter.from_processors(my_processors) +``` + +**After:** +```python +from pydantic_ai.capabilities import ProcessHistory +caps = [ProcessHistory(p) for p in my_processors] +``` + +## 3. PromptInjectionManager + +**No changes needed.** The native agent path already uses `PendingMessageDrainCapability` for follow-up queue delivery. `inject()`/`consume()` remains for tool result augmentation. `queue()`/`pop_queued()`/`flush_pending_to_queue()` remains for ACP agents only. + +## 4. ToolKind + +**Status**: Deprecated. Use string-based tool name patterns for config validation. + +**Before:** +```python +from agentpool.tools.base import ToolKind +tool = FunctionTool(name="bash", description="d", callable=fn, category="execute") +``` + +**After:** +```python +tool = FunctionTool(name="bash", description="d", callable=fn) +# Use tool name patterns for validation: allowed_tools: ["bash", "read"] +``` + +## 5. ToolResult.structured_content + +**Status**: Deprecated. Use PydanticAI's `ToolReturn` natively. + +**Before:** +```python +result = ToolResult(content="summary", structured_content={"key": "value"}) +``` + +**After:** +```python +from pydantic_ai.messages import ToolReturn +return ToolReturn(content="summary", structured_content={"key": "value"}) +``` + +## 6. Event Subclasses (PartStartEvent/PartDeltaEvent) + +**Status**: Deprecated. `session_id` should be accessed via `AgentContext` or `RunContext.deps`. + +**Before:** +```python +event = PartStartEvent(index=0, part=TextPart(content="hi")) +session_id = event.session_id +``` + +**After:** +```python +# Use PydanticAI's event directly +from pydantic_ai import PartStartEvent +event = PartStartEvent(index=0, part=TextPart(content="hi")) +# Get session_id from context, not from event payload +session_id = run_ctx.session_id +``` + +## Timeline + +- **Current**: Deprecation warnings emitted, all old APIs functional +- **v0.5.0**: Deprecated APIs removed diff --git a/docs/rfcs/draft/RFC-0001-nativeagent-pydantic-ai-refactor.md b/docs/rfcs/draft/RFC-0001-nativeagent-pydantic-ai-refactor.md new file mode 100644 index 000000000..57b9effdc --- /dev/null +++ b/docs/rfcs/draft/RFC-0001-nativeagent-pydantic-ai-refactor.md @@ -0,0 +1,575 @@ +--- +rfc_id: RFC-0001 +status: DRAFT +author: Sisyphus (AI Agent) +created: 2026-06-01 +last_updated: 2026-06-01 +--- + +# RFC-0001: 将 NativeAgent 精简为 pydantic-ai 模式 + +## Overview + +本 RFC 提出将 `agentpool` 中的 `NativeAgent` 从当前的 "wrapper over wrapper" 架构重构为直接基于 `pydantic-ai` 原生模式的轻量实现。当前 `NativeAgent` 在 `BaseAgent` 中自建了完整的 agent loop、injection manager、event system、tool framework，然后每次 run 时才临时构造 `pydantic_ai.Agent` 作为底层执行引擎。这种架构导致大量代码重复、维护成本高、且 pydantic-ai 的新特性无法及时暴露。 + +本 RFC 的目标是让 `NativeAgent` 成为 `pydantic-ai Agent` 的薄层适配器，将 turn 内部的 model loop、tool calling、output validation、streaming 等职责完全委托给 pydantic-ai，agentpool 仅保留 YAML 配置、session 管理、协议暴露、多 agent 编排等独特价值。 + +## Background & Context + +### 当前架构 + +``` +┌─────────────────────────────────────────────────────────────┐ +│ Protocol Handlers (ACP / OpenCode / AG-UI / MCP / API) │ +├─────────────────────────────────────────────────────────────┤ +│ SessionPool (TurnRunner + EventBus + SessionController) │ +├─────────────────────────────────────────────────────────────┤ +│ AgentPool Registry (YAML Manifest + 异构 Agent 管理) │ +├─────────────────────────────────────────────────────────────┤ +│ BaseAgent │ +│ ├── run_stream() — while loop + injection_manager │ +│ ├── ToolManager — 自建 tool framework │ +│ ├── MessageHistory — 自建 conversation management │ +│ ├── AgentRunContext — per-run state + cancellation │ +│ └── _stream_events() — abstract, subclass implements │ +├─────────────────────────────────────────────────────────────┤ +│ NativeAgent │ +│ └── get_agentlet() — 每次 run new 一个 PydanticAgent │ +│ ├── 收集 tools → wrap → to_pydantic_ai() │ +│ ├── 收集 instructions from providers │ +│ └── 构造 PydanticAgent(...) │ +├─────────────────────────────────────────────────────────────┤ +│ pydantic-ai Agent (临时实例，run 完即弃) │ +│ └── Agent.iter() → AgentRun → Graph Loop │ +│ (UserPromptNode → ModelRequestNode → CallToolsNode) │ +└─────────────────────────────────────────────────────────────┘ +``` + +### 问题分析 + +1. **双重 loop**：BaseAgent 有 while loop 处理 queue/injection，PydanticAgent 内部也有 graph loop 处理 model→tool→model。两层嵌套导致复杂度高。 + +2. **临时实例**：每次 `run()` 都 `new PydanticAgent()`，tool wrapping、instruction 收集重复执行，无法复用。 + +3. **自建轮子**： + - `Tool[T]` / `FunctionTool` / `ToolManager` → pydantic-ai 已有 `Tool` / `FunctionToolset` / `ToolManager` + - `MessageHistory` + compaction → pydantic-ai 已有 `message_history: list[ModelMessage]` + `HistoryProcessor` + - `AgentRunContext` → pydantic-ai 已有 `RunContext` + `GraphRunContext` + - `RichAgentStreamEvent` → pydantic-ai 已有 `AgentStreamEvent` / `FunctionToolCallEvent` / `FunctionToolResultEvent` + - `to_structured()` 运行时突变 → pydantic-ai 的 `output_type` + validators 更干净 + +4. **特性滞后**：pydantic-ai 持续演进（graph builder、capabilities、pending messages、durable execution），agentpool 无法自动继承。 + +## Problem Statement + +### 具体问题 + +- **维护负担高**：BaseAgent + NativeAgent 约 2000+ 行代码，大量逻辑与 pydantic-ai 重复 +- **类型不安全**：自建 event system 缺乏 pydantic-ai 的严格类型保证 +- **测试覆盖难**：自建 loop 的 edge cases（cancellation、injection timing、tool retry）需要大量测试 +- **功能受限**：无法使用 pydantic-ai 的 `Agent.iter()` 粒度控制、`GraphBuilder` 工作流、`capabilities` 插件系统 + +### 不解决的代价 + +- 每次 pydantic-ai 升级都需要在 agentpool 中做适配映射 +- 新开发者需要同时理解两套 loop 语义 +- AgentPool 的"重"使其难以嵌入其他项目 + +## Goals & Non-Goals + +### Goals + +1. **NativeAgent 成为 pydantic-ai Agent 的持久持有者**，而非临时构造器 +2. **BaseAgent 剥离 turn 内部 loop**，仅保留跨 turn 的 session/queuing 逻辑 +3. **Tool 体系复用 pydantic-ai 原生实现**，保留 agentpool 特有的 MCP bridge 和 ToolResult enrichment +4. **Event 体系映射到 pydantic-ai 原生事件**，在 SessionPool/Protocol 层做 enrichment +5. **MessageHistory 复用 pydantic-ai 的 `ModelMessage`**，保留 persistence/compaction 层 + +### Non-Goals + +1. **不动 ACPAgent / ClaudeCodeAgent / CodexAgent / AGUIAgent**：这些 agent 没有 LLM loop，不涉及 pydantic-ai 迁移 +2. **不动 SessionPool / Protocol Handlers**：这些层不直接调用 LLM +3. **不动 YAML Config / AgentPool Registry**：配置层是 agentpool 的核心价值 +4. **不引入 pydantic-graph 重写 Team/TeamRun**：本 RFC 仅聚焦 NativeAgent +5. **不追求 100% API 兼容**：允许 Breaking Changes，但需有迁移指南 + +## Evaluation Criteria + +| Criterion | Weight | Description | +|---|---|---| +| **Code Reduction** | High | NativeAgent + BaseAgent 代码行数减少目标 ≥ 50% | +| **Maintainability** | High | 移除自建 loop 后的测试复杂度、debug 难度 | +| **Feature Parity** | High | 现有功能（streaming、injection、queuing、tools、history、hooks）不丢失 | +| **Pydantic-AI Alignment** | High | 能自动继承 pydantic-ai 新特性（capabilities、graph、evals） | +| **Breaking Impact** | Medium | 对外部用户 API 的影响范围、迁移成本 | +| **Implementation Risk** | Medium | 重构期间的功能回归风险、测试覆盖要求 | + +## Options Analysis + +### Option 1: 保守适配 — 保持 BaseAgent，NativeAgent 持有 PydanticAgent 实例 + +**Description**： +- `BaseAgent` 保持不变，仍提供 `run_stream()` while loop + injection manager +- `NativeAgent.__init__()` 中直接构造 `PydanticAgent` 并持久持有 +- `NativeAgent._stream_events()` 调用 `self._pydantic_agent.iter()`，但仍在 background task 中运行，通过 queue 中转事件 +- 事件转换：pydantic-ai `AgentStreamEvent` → agentpool `RichAgentStreamEvent` + +**Advantages**： +- 改动范围最小，主要集中在 `native_agent/agent.py` +- BaseAgent 的 queuing/injection 语义保持不变，上层（SessionPool/Protocol）无感知 +- 风险低，可渐进式验证 + +**Disadvantages**： +- 仍存在双重 loop（BaseAgent while + pydantic-ai graph），复杂度未根本降低 +- BaseAgent 的 1000+ 行代码无法删除 +- 无法使用 pydantic-ai 的 `RunContext.enqueue()` 替代 injection manager + +**Evaluation**： +| Criterion | Score | Notes | +|---|---|---| +| Code Reduction | 2/5 | 仅 NativeAgent 减少，BaseAgent 不变 | +| Maintainability | 2/5 | 双重 loop 仍在 | +| Feature Parity | 5/5 | 无功能损失 | +| Pydantic-AI Alignment | 2/5 | 仍隔离在 wrapper 层 | +| Breaking Impact | 5/5 | 无 Breaking Change | +| Implementation Risk | 4/5 | 低风险 | + +**Effort Estimate**：1-2 周，1 人 + +--- + +### Option 2: 激进重构 — BaseAgent 拆分为 BaseNode + TurnController，NativeAgent 完全 delegate 给 pydantic-ai + +**Description**： +- 将 `BaseAgent` 拆分为两个角色： + - **`BaseNode`**：保留 MessageNode 接口（`process()`、`connections`、`signals`），但移除 `run_stream()` while loop + - **`TurnController`**（或并入 `SessionPool.TurnRunner`）：接管 queuing/injection/auto-resume +- `NativeAgent` 直接继承 `BaseNode`，内部持有 `PydanticAgent` +- `NativeAgent.run()` = 调用 `self._pydantic_agent.run_sync()` 或 `run()` +- `NativeAgent.run_stream()` = 调用 `self._pydantic_agent.iter()`，直接 yield pydantic-ai events（或做 thin mapping） +- **Injection/Queuing**：从 NativeAgent 层移除，完全由 SessionPool/TurnRunner 处理。Tool 中的 `ctx.agent.inject_prompt()` 改为 `ctx.run_context.enqueue()`（pydantic-ai native）或由 SessionPool 调度 + +**Advantages**： +- 根本消除双重 loop，架构清晰 +- BaseAgent 代码可减少 60%+ +- 完全对齐 pydantic-ai 语义，自动继承新特性 +- 可使用 pydantic-ai `Agent.iter()` 的细粒度控制（逐 node 观察） + +**Disadvantages**： +- **Breaking Change**：`BaseAgent.run_stream()` 的 while loop 被移除，影响所有子类（ACPAgent 等需要适配） +- **Injection 语义变化**：`agent.inject_prompt()` 不再可用（或需要 SessionPool 层模拟），影响现有 tool 实现 +- **Event 体系变化**：`RichAgentStreamEvent` 需要重新设计为 pydantic-ai events 的 enrichment +- 测试重写工作量大 + +**Evaluation**： +| Criterion | Score | Notes | +|---|---|---| +| Code Reduction | 5/5 | BaseAgent + NativeAgent 大幅精简 | +| Maintainability | 5/5 | 单一层级 loop | +| Feature Parity | 3/5 | injection/queuing 语义需重新设计 | +| Pydantic-AI Alignment | 5/5 | 完全对齐 | +| Breaking Impact | 2/5 | 影响所有 Agent 子类和 tool 实现 | +| Implementation Risk | 2/5 | 高风险，需充分测试 | + +**Effort Estimate**：4-6 周，1-2 人 + +--- + +### Option 3: 混合方案 — BaseAgent 保留接口但内部 delegate loop 到 pydantic-ai + +**Description**： +- `BaseAgent` 保留 `run_stream()` 接口签名，但内部不再自建 while loop +- `BaseAgent.run_stream()` 改为调用 `_run_single_turn()`（由子类实现） +- `NativeAgent._run_single_turn()` = 调用 `self._pydantic_agent.iter()` +- `BaseAgent` 的 queuing/injection 保留，但实现改为： + - `queue_prompt()` → 将 prompt 加入 SessionPool 队列（或 pydantic-ai `RunContext.enqueue()`） + - `inject_prompt()` → 同上，或标记为 deprecated，推荐 SessionPool 调度 +- 非 NativeAgent 子类（ACPAgent）继续用自己的 `_run_single_turn()` 实现 + +**Advantages**： +- 保留 `BaseAgent.run_stream()` 接口，子类无需立即适配 +- NativeAgent 获得 pydantic-ai 完整 loop，其他 agent 不受影响 +- 可以渐进式移除 BaseAgent 的 loop 逻辑 + +**Disadvantages**： +- BaseAgent 仍需维护（虽然 loop 逻辑可简化） +- `run_stream()` 的 while loop 语义与 pydantic-ai `iter()` 的语义不完全一致，存在认知负担 +- 不是根本解决，是过渡方案 + +**Evaluation**： +| Criterion | Score | Notes | +|---|---|---| +| Code Reduction | 3/5 | BaseAgent 简化但保留 | +| Maintainability | 3/5 | 仍有接口层差异 | +| Feature Parity | 4/5 | 大部分保留，injection 需调整 | +| Pydantic-AI Alignment | 4/5 | NativeAgent 完全对齐 | +| Breaking Impact | 3/5 | 接口保留，内部实现变 | +| Implementation Risk | 3/5 | 中等风险 | + +**Effort Estimate**：2-3 周，1 人 + +## Recommendation + +**推荐 Option 2（激进重构）**，但采用分阶段实施以降低风险。 + +### 推荐理由 + +1. **根本解决问题**：Option 1 和 Option 3 都是"在现有架构上打补丁"，无法消除双重 loop 的根本矛盾。agentpool 的核心价值在 SessionPool/Protocol/Registry 层，不在 agent loop 层。 + +2. **长期维护成本**：pydantic-ai 是活跃维护的框架（Pydantic 团队），其 agent loop 的可靠性、测试覆盖、新特性演进远超 agentpool 自建实现。delegate 后维护成本显著降低。 + +3. **与 SessionPool 架构一致**：`ARCHITECTURE-ORCHESTRATOR.md` 设计文档已经明确"Turn = 单次 LLM 调用"，SessionPool 负责 turn 编排。将 turn 内部 loop 下沉到 pydantic-ai 与该设计完全一致。 + +4. **接受的风险可控**：Breaking Changes 主要影响内部 tool 实现和子类适配，外部用户（YAML 配置 + Protocol）感知有限。 + +### 接受的风险 + +- **Injection 语义变化**：现有 tool 中 `ctx.agent.inject_prompt()` 需要改为 `ctx.run_context.enqueue()` 或 SessionPool API。需编写迁移指南。 +- **Event 适配成本**：`RichAgentStreamEvent` 体系需要重写为 pydantic-ai events 的 enrichment 层。 +- **测试重写**：NativeAgent 的测试需要大量重写，但可复用 pydantic-ai 的 `TestModel`。 + +## Technical Design + +### 目标架构 + +``` +┌─────────────────────────────────────────────────────────────┐ +│ Protocol Handlers (ACP / OpenCode / AG-UI / MCP / API) │ +│ → 通过 EventBus 消费事件，不受 NativeAgent 内部变化影响 │ +├─────────────────────────────────────────────────────────────┤ +│ SessionPool (TurnRunner + EventBus + SessionController) │ +│ → TurnRunner 直接调用 agent.run() / agent.iter() │ +│ → 不再依赖 agent._run_stream_once() │ +├─────────────────────────────────────────────────────────────┤ +│ AgentPool Registry (YAML Manifest + 异构 Agent 管理) │ +├─────────────────────────────────────────────────────────────┤ +│ BaseNode (原 BaseAgent 精简) │ +│ ├── process() / run() / run_stream() — 纯接口 │ +│ ├── connections / signals — 保留 │ +│ └── conversation — 可简化为 thin wrapper │ +├─────────────────────────────────────────────────────────────┤ +│ NativeAgent[TDeps, OutputDataT] │ +│ ├── _pydantic_agent: PydanticAgent[TDeps, OutputDataT] │ +│ │ (init 时构造，持久持有) │ +│ ├── run() → self._pydantic_agent.run_sync() │ +│ ├── run_stream() → self._pydantic_agent.iter() → map events│ +│ └── tools → 注册到 PydanticAgent（支持 prepare/override） │ +├─────────────────────────────────────────────────────────────┤ +│ pydantic-ai Agent │ +│ └── Agent.iter() → AgentRun → Graph Loop │ +│ (UserPromptNode → ModelRequestNode → CallToolsNode) │ +│ └── ToolManager / FunctionToolset / output validators │ +│ └── RunContext / GraphRunContext / message_history │ +└─────────────────────────────────────────────────────────────┘ +``` + +### NativeAgent 新设计 + +```python +class NativeAgent[TDeps = None, TResult = str](BaseNode[TDeps, TResult]): + """AgentPool 原生 agent，基于 pydantic-ai Agent 构建。 + + 职责： + 1. 将 YAML 配置转换为 PydanticAgent 配置 + 2. 管理 tools（含 MCP bridge、agent-as-tool） + 3. 将 pydantic-ai events 映射为 agentpool events + 4. 管理 conversation persistence（在 pydantic-ai message_history 之上） + """ + + def __init__( + self, + *, + name: str, + model: str | Model, + system_prompt: str | Sequence[str] = (), + instructions: Sequence[str | Callable] = (), + tools: Sequence[Tool | Callable] = (), + toolsets: Sequence[AbstractToolset] = (), + output_type: type[TResult] = str, + deps_type: type[TDeps] = type(None), + model_settings: ModelSettings | None = None, + retries: int | AgentRetries | None = None, + end_strategy: EndStrategy = "early", + capabilities: Sequence[AgentCapability] = (), + # agentpool 特有 + agent_pool: AgentPool | None = None, + mcp_servers: Sequence[str | MCPServerConfig] = (), + hooks: AgentHooks | None = None, + storage: StorageManager | None = None, + ) -> None: + # 构建 pydantic-ai Agent（持久持有，非临时） + self._pydantic_agent = PydanticAgent( + model=model, + name=name, + system_prompt=system_prompt, + instructions=instructions, + tools=tools, + toolsets=toolsets, + output_type=output_type, + deps_type=deps_type, + model_settings=model_settings, + retries=retries, + end_strategy=end_strategy, + capabilities=capabilities, + ) + # agentpool 特有层 + self.agent_pool = agent_pool + self.hooks = hooks + self._storage = storage + # conversation persistence（在 pydantic-ai history 之上） + self._conversation_persistence = ConversationPersistence(storage) + + async def run( + self, + *prompts: PromptCompatible, + store_history: bool = True, + message_history: Sequence[ModelMessage] | None = None, + deps: TDeps | None = None, + **kwargs: Any, + ) -> ChatMessage[TResult]: + """Run agent and return final message.""" + # 从持久化加载历史 + history = message_history or await self._load_persisted_history() + # 直接委托给 pydantic-ai + result = await self._pydantic_agent.run( + prompts, + message_history=history, + deps=self._build_run_context(deps), + ) + # 持久化新消息 + if store_history: + await self._persist_messages(result.all_messages()) + return self._to_chat_message(result) + + async def run_stream( + self, + *prompts: PromptCompatible, + store_history: bool = True, + message_history: Sequence[ModelMessage] | None = None, + deps: TDeps | None = None, + **kwargs: Any, + ) -> AsyncIterator[RichAgentStreamEvent[TResult]]: + """Run agent with streaming events.""" + history = message_history or await self._load_persisted_history() + async with self._pydantic_agent.iter( + prompts, + message_history=history, + deps=self._build_run_context(deps), + ) as agent_run: + async for event in self._enrich_events(agent_run, session_id=self.session_id): + yield event + if store_history: + await self._persist_messages(agent_run.result.all_messages()) + + def _enrich_events( + self, + agent_run: AgentRun, + session_id: str | None = None, + ) -> AsyncIterator[RichAgentStreamEvent[TResult]]: + """将 pydantic-ai events 映射为 agentpool events，添加 session/agent 上下文。""" + # pydantic-ai AgentStreamEvent → agentpool RichAgentStreamEvent + # 添加：session_id、agent_name、cost_info、tool_call_id mapping + ... + + async def _build_run_context(self, user_deps: TDeps | None) -> RunContext[TDeps]: + """构建 pydantic-ai RunContext，注入 agentpool 特有依赖。""" + # 包含：AgentPool、InputProvider、internal_fs、hooks + ... +``` + +### Event 映射层 + +| pydantic-ai Event | agentpool RichAgentStreamEvent | Enrichment | +|---|---|---| +| `PartStartEvent` | `PartStartEvent` | + agent_name, session_id | +| `PartDeltaEvent` | `PartDeltaEvent` | + agent_name, session_id | +| `PartEndEvent` | `PartEndEvent` | + agent_name, session_id | +| `FunctionToolCallEvent` | `ToolCallStartEvent` | + agent_name, session_id, tool_call_id | +| `FunctionToolResultEvent` | `ToolCallCompleteEvent` | + agent_name, session_id, metadata | +| `FinalResultEvent` | `StreamCompleteEvent` | + agent_name, session_id, cost_info, ChatMessage | +| `ModelResponse` (node) | `ModelResponseEvent` | + model_name, provider_name, usage | +| `RunUsage` | — | 累积到 ChatMessage.cost_info | + +### Tool 适配 + +```python +# agentpool Tool → pydantic-ai Tool +class AgentpoolToolAdapter: + """将 agentpool 的 Tool/FunctionTool 适配为 pydantic-ai Tool。 + + 保留 agentpool 特有功能： + - schema_override（schemez） + - requires_confirmation（通过 pydantic-ai ApprovalRequiredToolset） + - ToolResult enrichment（content + structured_content + metadata） + - MCP bridge（通过 pydantic-ai MCPServerTool / ExternalToolset） + """ + + @staticmethod + def to_pydantic_ai(tool: Tool, context: AgentContext) -> PydanticTool: + # 1. 提取 callable + fn = tool.get_callable() + # 2. 应用 schema_override（schemez） + schema = tool.schema_override or infer_schema(fn) + # 3. 包装为 pydantic-ai Tool + pydantic_tool = PydanticTool( + fn, + name=tool.name, + description=tool.description, + # prepare 函数处理 schema_override + prepare=tool._get_effective_prepare(), + ) + # 4. confirmation 通过 ApprovalRequiredToolset 处理 + if tool.requires_confirmation: + pydantic_tool = ApprovalRequiredToolset([pydantic_tool]) + return pydantic_tool +``` + +### MessageHistory 适配 + +```python +class ConversationPersistence: + """在 pydantic-ai message_history 之上提供持久化和 compaction。 + + pydantic-ai 负责： + - 运行时 message_history: list[ModelMessage] + - HistoryProcessor（pre-run 处理） + + agentpool 负责： + - 加载/保存到 StorageManager（SQL） + - Compaction/summarization（跨 session） + - format_history()（用于非 LLM 消费） + """ + + async def load(self, session_id: str) -> list[ModelMessage]: + # 从 SQL 加载 ChatMessage，转换为 ModelMessage + ... + + async def save(self, session_id: str, messages: list[ModelMessage]) -> None: + # 将 ModelMessage 转换为 ChatMessage，保存到 SQL + ... + + async def compact(self, session_id: str, max_tokens: int) -> list[ModelMessage]: + # 调用 compaction 策略，返回压缩后的 history + ... +``` + +## Implementation Plan + +### Phase 1: 基础设施准备（1 周） + +1. **Event 映射层** + - 实现 `_enrich_events()`：pydantic-ai `AgentStreamEvent` → `RichAgentStreamEvent` + - 确保所有现有 event types 都有对应映射 + - 编写 event 映射测试 + +2. **Tool 适配层** + - 实现 `AgentpoolToolAdapter.to_pydantic_ai()` + - 验证 schema_override、prepare、confirmation 功能 + - MCP bridge 验证 + +3. **MessageHistory 适配层** + - 实现 `ConversationPersistence` + - `ChatMessage` ↔ `ModelMessage` 双向转换 + - Compaction 逻辑迁移 + +### Phase 2: NativeAgent 重构（1-2 周） + +1. **新 NativeAgent 实现** + - 基于 `BaseNode`（精简后的基类） + - 持久持有 `PydanticAgent` + - 实现 `run()` / `run_stream()` / `run_iter()` + +2. **BaseAgent 精简** + - 移除 while loop（移至 SessionPool/TurnRunner） + - 移除 injection manager（使用 pydantic-ai `RunContext.enqueue()` 或 SessionPool 队列） + - 保留：signals、connections、hooks interface + +3. **测试覆盖** + - 使用 pydantic-ai `TestModel` 编写单元测试 + - 集成测试：streaming、tool calling、output validation、history + - 回归测试：现有 YAML config 加载运行 + +### Phase 3: 子类适配（1 周） + +1. **ACPAgent 适配** + - ACPAgent 不依赖 pydantic-ai，但需要适配精简后的 BaseNode 接口 + - 验证 `_stream_events()` 接口变化 + +2. **其他子类** + - ClaudeCodeAgent、CodexAgent、AGUIAgent 验证 + +### Phase 4: SessionPool 集成（1 周） + +1. **TurnRunner 适配** + - 调用 `agent.run()` / `agent.iter()` 替代 `_run_stream_once()` + - auto-resume 逻辑验证 + +2. **EventBus 适配** + - 消费 pydantic-ai 原生 events（经 enrichment） + +### Phase 5: 文档与迁移（1 周） + +1. **API 文档更新** +2. **迁移指南** + - `agent.inject_prompt()` → `RunContext.enqueue()` + - `agent.queue_prompt()` → SessionPool API + - Event handler 类型变化 +3. **内部工具迁移审查** + +### Rollback Strategy + +- 每个 Phase 都有独立分支 +- Phase 2-3 期间保留旧 NativeAgent 为 `NativeAgentLegacy`，通过 feature flag 切换 +- 全部验证通过后删除 Legacy + +## Open Questions + +1. **pydantic-ai 版本**：当前 agentpool 依赖 PyPI 版 `pydantic-ai-slim>=1.0.0`，是否改为 editable 依赖本地 `packages/pydantic-ai/`？ +2. **`AgentContext` 如何处理**：pydantic-ai 的 `RunContext` 类型参数是 `deps_type`，agentpool 的 `AgentContext` 包含 pool、input_provider、fs 等。如何在不破坏类型安全的前提下注入？ +3. **Hooks 体系**：`AgentHooks` 的 pre_run/post_run hooks 如何在 pydantic-ai 的 capability/hook 体系中实现？是否需要实现 custom capability？ +4. **Session ID / Conversation ID**：pydantic-ai 的 `conversation_id` 是 `uuid7`，agentpool 使用字符串 session_id。映射策略？ +5. **`to_structured()` 运行时突变**：现有代码允许运行时改变 output_type。 pydantic-ai 是否支持？如果不支持，替代方案？ + +## Decision Record + +| Field | Value | +|---|---| +| **Decision** | 采用 Option 2（激进重构），分 5 个 Phase 实施 | +| **Date** | 2026-06-01 | +| **Approver** | 待确定 | +| **Key Discussion Points** | 1. Breaking Change 的接受程度；2. Injection 语义迁移成本；3. 与 pydantic-ai 版本绑定策略 | +| **Conditions** | 1. 本地 `packages/pydantic-ai/` 保持活跃同步；2. 每个 Phase 有独立 rollback 能力；3. 现有 YAML config 100% 兼容 | + +--- + +## Appendix A: 代码行数估算 + +| 模块 | 当前行数 | 目标行数 | 减少比例 | +|---|---|---|---| +| `agents/base_agent.py` | ~1224 | ~400 | 67% | +| `agents/native_agent/agent.py` | ~1341 | ~500 | 63% | +| `tools/base.py` | ~787 | ~300 | 62% | +| `messaging/message_history.py` | ~347 | ~150 | 57% | +| **合计** | **~3700** | **~1350** | **63%** | + +## Appendix B: 相关文件清单 + +### 需要重构的文件 +- `src/agentpool/agents/base_agent.py` — 精简 while loop、injection manager +- `src/agentpool/agents/native_agent/agent.py` — 改为持有 PydanticAgent +- `src/agentpool/agents/native_agent/tool_wrapping.py` — 适配 pydantic-ai Tool +- `src/agentpool/tools/base.py` — 简化，delegate schema 到 pydantic-ai +- `src/agentpool/messaging/message_history.py` — 改为 persistence 层 +- `src/agentpool/agents/events/` — 添加 event mapping 层 + +### 需要适配的文件 +- `src/agentpool/orchestrator/core.py` — TurnRunner 调用新接口 +- `src/agentpool/agents/acp_agent/acp_agent.py` — 适配精简后的基类 +- `src/agentpool/agents/claude_code_agent/` — 验证兼容性 +- `src/agentpool/agents/codex_agent/` — 验证兼容性 + +### 不需要改动的文件 +- `src/agentpool/delegation/pool.py` — Registry 层不变 +- `src/agentpool_server/` — Protocol 层不变 +- `src/agentpool/models/manifest.py` — YAML schema 不变 +- `src/agentpool/sessions/` — Session persistence 不变 diff --git a/openspec/changes/pydanticai-thinning-refactor/.openspec.yaml b/openspec/changes/pydanticai-thinning-refactor/.openspec.yaml new file mode 100644 index 000000000..34f9314d2 --- /dev/null +++ b/openspec/changes/pydanticai-thinning-refactor/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-06-29 diff --git a/openspec/changes/pydanticai-thinning-refactor/design.md b/openspec/changes/pydanticai-thinning-refactor/design.md new file mode 100644 index 000000000..cb50b2265 --- /dev/null +++ b/openspec/changes/pydanticai-thinning-refactor/design.md @@ -0,0 +1,128 @@ +## Context + +AgentPool is built on PydanticAI, using a composition pattern: `get_agentlet()` creates a fresh `pydantic_ai.Agent` at runtime with capabilities assembled from tool providers, hooks, MCP servers, skills, etc. The integration surface is ~500 lines of bridge code (`as_capability()`, `get_agentlet()`, `to_pydantic_ai()`, `wrap_tool()`). + +Over time, AgentPool accumulated custom abstractions that overlap with PydanticAI's evolving API: + +1. **Hooks**: AgentPool has a 3-type hierarchy (`CallableHook`/`CommandHook`/`PromptHook`) with regex matchers, timeouts, and parallel result combining. PydanticAI now provides `pydantic_ai.capabilities.Hooks` with typed callbacks (`before_run`/`after_run`/`before_tool_execute`/`after_tool_execute`). +2. **ProcessHistory**: AgentPool has `ProcessHistoryAdapter` (~200 lines with caching + signature validation). PydanticAI provides `pydantic_ai.capabilities.ProcessHistory`. +3. **PromptInjectionManager**: For native agents, `queue()`/`pop_queued()` is redundant with `PendingMessageDrainCapability` (already the spec'd mechanism per `pending-message-queue`). +4. **Tool metadata**: `Tool` dataclass carries `ToolKind` taxonomy, `ToolResult.structured_content`, and other fields PydanticAI's `Tool` already provides. +5. **Event subclasses**: `PartStartEvent`/`PartDeltaEvent` subclass PydanticAI's versions just to add `session_id`. + +Current state: ~1,500 lines of custom code that duplicates PydanticAI functionality. + +## Goals / Non-Goals + +**Goals:** +- Remove ~1,000-1,500 lines of code that duplicates PydanticAI functionality +- Delegate agent-engine concerns to PydanticAI (hooks, history processing, tool definition, event types) +- Keep AgentPool "thick" only at the orchestration layer (sessions, multi-agent, protocols, skills) +- Maintain behavioral parity — all existing test scenarios must pass +- Reduce coupling to `pydantic_ai._internal` APIs + +**Non-Goals:** +- Removing ACP agent support (the manual queue system for ACP stays) +- Changing the `ResourceProvider` hierarchy (it's AgentPool's value-add) +- Changing the Skills system (implements Agent Skills Spec, no PydanticAI equivalent) +- Changing `SessionPool`/`SessionController`/`EventBus`/`TurnRunner` (orchestration layer stays) +- Changing YAML graph compiler or pydantic-graph integration +- Changing protocol servers (ACP/AG-UI/OpenCode/OpenAI API) + +## Decisions + +### D1: Hooks — Delegate to `pydantic_ai.capabilities.Hooks`, keep Command/Prompt as thin adapters + +**Choice**: Migrate `NativeAgentHookManager.as_capability()` to directly produce a `pydantic_ai.capabilities.Hooks` instance with typed callbacks. Remove `Hook` base class, `CallableHook`, regex matchers, timeout handling, and parallel result combining. `CommandHook` and `PromptHook` survive as thin adapters that implement `Hooks` callbacks internally. + +**Rationale**: PydanticAI's `Hooks` provides the same 4 hook points (`before_run`/`after_run`/`before_tool_execute`/`after_tool_execute`) with a cleaner API. AgentPool's parallel result combining (deny > ask > allow) can be replicated inside a single `Hooks` callback that aggregates multiple registered hooks. `CommandHook` (subprocess evaluation) and `PromptHook` (LLM evaluation) are unique to AgentPool and have no PydanticAI equivalent — they become thin wrappers that register their logic as `Hooks` callbacks. + +**Alternatives considered**: +- *Keep all custom hooks, just wrap them*: Rejected — defeats the purpose of thinning and adds an extra layer. +- *Remove CommandHook and PromptHook entirely*: Rejected — they provide unique capabilities (subprocess + LLM evaluation) not available in PydanticAI. + +### D2: ProcessHistory — Use `pydantic_ai.capabilities.ProcessHistory` directly + +**Choice**: Remove `ProcessHistoryAdapter`. Register custom history processors (compaction, etc.) as callbacks on PydanticAI's `ProcessHistory` capability. The caching and signature validation in `ProcessHistoryAdapter` is dropped — PydanticAI handles lifecycle internally. + +**Rationale**: `ProcessHistoryAdapter` was written before PydanticAI had a stable `ProcessHistory` capability. The caching layer was needed because AgentPool rebuilt the agent per-run, but PydanticAI's `ProcessHistory` already handles this efficiently. Signature validation was for detecting config changes — this is now handled by the capability rebuild mechanism in `get_agentlet()`. + +**Alternatives considered**: +- *Keep ProcessHistoryAdapter as a thin wrapper*: Rejected — it would just forward calls to PydanticAI's `ProcessHistory` with no added value. + +### D3: PromptInjectionManager — Remove native path, keep ACP-only + +**Choice**: `PromptInjectionManager.inject()`/`consume()` (tool result augmentation) is preserved for all agents. `queue()`/`pop_queued()`/`flush_pending_to_queue()` are removed for native agents — native agents fully use `PendingMessageDrainCapability` for follow-up queue. `PromptInjectionManager` survives only as the ACP-agent manual queue. + +**Rationale**: The `pending-message-queue` spec already established that native agents use `PendingMessageDrainCapability` for follow-up delivery. The `queue()`/`pop_queued()` methods on `PromptInjectionManager` are dead code for native agents — they're never called because `RunExecutor` drives `agent_run.next(node)` which triggers `PendingMessageDrainCapability` hooks. The `inject()`/`consume()` path (tool result augmentation via `` tags) is NOT replaced by PydanticAI — it modifies tool results, not conversation messages. + +**Alternatives considered**: +- *Remove PromptInjectionManager entirely*: Rejected — ACP agents still need the manual queue, and tool result augmentation is unique to AgentPool. +- *Keep everything as-is*: Rejected — the native path is dead code that confuses maintainers. + +### D4: Tool dataclass — Thin wrapper, remove redundant metadata + +**Choice**: Remove `ToolKind` taxonomy (PydanticAI has no equivalent and it's unused outside config validation). Remove `ToolResult.structured_content` (PydanticAI's `ToolReturn` already supports structured returns). Simplify `Tool.to_pydantic_ai()` to a direct 1:1 mapping. Tool confirmation uses PydanticAI's `requires_approval` where possible; `ApprovalRequiredToolset` stays for deferred execution scenarios. + +**Rationale**: `ToolKind` was a categorization system for tool permissions, but it's only used in config validation, not at runtime. `ToolResult.structured_content` duplicates PydanticAI's native structured return. The 60-line `to_pydantic_ai()` conversion handles edge cases that no longer exist after simplification. + +**Alternatives considered**: +- *Keep ToolKind for config validation*: Rejected — validation can use string matching on tool names instead. +- *Remove Tool dataclass entirely, use PydanticAI Tool*: Rejected — AgentPool needs `AgentContext` injection, which PydanticAI's `RunContext` doesn't provide natively. The `wrap_tool()` adapter is still needed. + +### D5: Events — Stop subclassing PydanticAI events, pass session_id via context + +**Choice**: Remove `PartStartEvent(PyAIPartStartEvent)` and `PartDeltaEvent(PyAIPartDeltaEvent)` subclasses. Use PydanticAI's `AgentStreamEvent` types directly. `session_id` is passed via `RunContext.deps` (which already carries `AgentContext` containing `session_id`). `ToolCallStartEvent`/`ToolCallCompleteEvent` become thin wrappers over PydanticAI's `FunctionToolCallEvent`/`FunctionToolResultEvent`, constructed in `RunExecutor` without subclassing. + +**Rationale**: Subclassing PydanticAI events just to add `session_id` creates coupling — every PydanticAI event change requires AgentPool to update subclasses. `session_id` is already available via `AgentContext` in `RunContext.deps`. Protocol consumers that need `session_id` can get it from the context, not the event payload. + +**Alternatives considered**: +- *Keep subclasses, just add session_id to all*: Rejected — requires maintaining subclasses for every PydanticAI event type, now and in the future. +- *Wrap events in an envelope*: Rejected — `EventEnvelope` on `EventBus` already provides session_id routing; duplicating it on the event itself is redundant. + +## Risks / Trade-offs + +- **[Risk] Hook behavior divergence**: AgentPool's parallel hook combining (deny > ask > allow) may behave differently from sequential `Hooks` callbacks. → **Mitigation**: Implement combining logic inside the `Hooks` callback wrapper, preserving exact priority semantics. Add regression tests for all hook combination scenarios before migration. + +- **[Risk] ProcessHistory caching loss**: Removing `ProcessHistoryAdapter`'s caching may impact performance for agents with many history processors. → **Mitigation**: Benchmark before/after. If impact is significant, add caching at the PydanticAI `ProcessHistory` callback level. + +- **[Risk] Breaking YAML configs**: Hook config schema changes (`matcher`/`event`/`timeout` → callback references) break existing configs. → **Mitigation**: Provide a config migration script. Document the migration in CHANGELOG. Keep a deprecation period where old config format is auto-translated. + +- **[Risk] Event consumer breakage**: Protocol servers that read `session_id` from event payload will break. → **Mitigation**: Audit all `event.session_id` access points. Replace with `run_ctx.session_id` lookups. Add type errors to catch missed access points. + +- **[Risk] ToolKind removal breaks config validation**: Configs using `kind: read` / `kind: edit` will fail validation. → **Mitigation**: Replace `kind` validation with string-based tool name patterns. Provide migration guide. + +## Migration Plan + +### Phase 1: Hooks Migration (highest impact, highest risk) +1. Write regression tests for all existing hook combination scenarios +2. Implement `HooksCapabilityAdapter` that wraps multiple AgentPool hooks into a single `pydantic_ai.capabilities.Hooks` +3. Migrate `CallableHook` to use `Hooks` callbacks +4. Migrate `CommandHook` and `PromptHook` as thin adapters +5. Remove `Hook` base class, regex matchers, timeout handling +6. Update YAML config schema with deprecation shim for old format + +### Phase 2: ProcessHistory + PromptInjectionManager +1. Replace `ProcessHistoryAdapter` with PydanticAI `ProcessHistory` +2. Remove `PromptInjectionManager.queue()`/`pop_queued()` for native agent path +3. Keep `inject()`/`consume()` for tool result augmentation +4. Keep `PromptInjectionManager` ACP manual queue intact + +### Phase 3: Tool + Event Thinning +1. Remove `ToolKind` taxonomy +2. Remove `ToolResult.structured_content` +3. Simplify `Tool.to_pydantic_ai()` to direct mapping +4. Remove `PartStartEvent`/`PartDeltaEvent` subclasses +5. Replace `event.session_id` access with context lookups +6. Simplify `RunExecutor` event mapping + +### Rollback Strategy +- Each phase is independently revertable via git +- Phase 1 can be rolled back without affecting Phase 2/3 +- If hook migration reveals behavioral divergence, restore `NativeAgentHookManager` from git + +## Open Questions + +1. **Hook config migration**: Should we provide an automatic YAML config migration script, or just document the new format? (Recommend: document + deprecation shim) +2. **ToolKind replacement**: Is string-based tool name pattern matching sufficient for config validation, or do we need a replacement taxonomy? (Recommend: string patterns, remove taxonomy) +3. **ProcessHistory benchmarking**: Should we benchmark before starting Phase 2, or trust that PydanticAI's implementation is efficient enough? (Recommend: quick benchmark first) diff --git a/openspec/changes/pydanticai-thinning-refactor/proposal.md b/openspec/changes/pydanticai-thinning-refactor/proposal.md new file mode 100644 index 000000000..56b3c37b7 --- /dev/null +++ b/openspec/changes/pydanticai-thinning-refactor/proposal.md @@ -0,0 +1,39 @@ +## Why + +AgentPool currently carries ~1,500 lines of custom code that duplicates functionality PydanticAI already provides (Hooks hierarchy, ProcessHistoryAdapter, PromptInjectionManager native path, Tool metadata中间层). This overlap increases maintenance burden, creates subtle behavioral divergences from upstream PydanticAI, and makes it harder to benefit from future PydanticAI improvements. The framework should be "thin" at the agent-engine layer (where PydanticAI is the source of truth) and "thick" only at the orchestration layer (where AgentPool provides unique value: sessions, multi-agent composition, protocol servers, skills). + +## What Changes + +- **Replace custom Hooks system with `pydantic_ai.capabilities.Hooks`**: Remove the 3-type Hook hierarchy (`CallableHook`/`CommandHook`/`PromptHook`), regex matchers, timeout handling, and parallel result combining. Migrate to PydanticAI's `Hooks` capability with typed callbacks (`before_run`/`after_run`/`before_tool_execute`/`after_tool_execute`). `CommandHook` and `PromptHook` become thin wrappers that internally delegate to `Hooks` callbacks. **BREAKING**: Hook config YAML schema changes (no more `matcher`/`event`/`timeout` fields; replaced by `before`/`after` callback references). + +- **Replace `ProcessHistoryAdapter` with PydanticAI's `ProcessHistory` capability**: Remove the custom `ProcessHistoryAdapter` implementation (~200 lines with caching + signature validation). Use `pydantic_ai.capabilities.ProcessHistory` directly. Custom history processors (compaction, etc.) are registered as `ProcessHistory` callbacks instead. + +- **Remove `PromptInjectionManager` for native agent path**: Native agents already use `PendingMessageDrainCapability` for follow-up queue (per `pending-message-queue` spec). The `inject()`/`consume()` tool-result augmentation path is preserved, but `queue()`/`pop_queued()`/`flush_pending_to_queue()` are removed for native agents. `PromptInjectionManager` survives only as the ACP-agent manual queue. + +- **Simplify `Tool` dataclass to thin wrapper**: Remove `ToolKind` taxonomy, `ToolResult.structured_content`, and redundant metadata fields that PydanticAI's `Tool` already provides. `Tool.to_pydantic_ai()` becomes a direct 1:1 mapping instead of a 60-line conversion. Tool confirmation uses PydanticAI's `requires_approval` natively instead of the custom `ApprovalRequiredToolset` wrapper where possible. + +- **Simplify `RunExecutor` event mapping**: PydanticAI's `AgentStreamEvent` already includes `PartStartEvent`/`PartDeltaEvent`/`FunctionToolCallEvent`. AgentPool's custom subclasses (`PartStartEvent`+`session_id`, `PartDeltaEvent`+`session_id`) are replaced by passing `session_id` via context, not by subclassing. `ToolCallStartEvent`/`ToolCallCompleteEvent` become thin wrappers over PydanticAI's `FunctionToolCallEvent`/`FunctionToolResultEvent`. + +- **Keep unchanged**: `SessionPool`, `SessionController`, `EventBus`, `TurnRunner` (ACP path), `AgentPool` registry, Skills system, ResourceProvider hierarchy, ACP agent, protocol servers, YAML graph compiler, pydantic-graph integration. + +## Capabilities + +### New Capabilities + +- `pydanticai-hooks-delegation`: AgentPool delegates hook lifecycle to PydanticAI's `Hooks` capability; custom hook types become thin adapters +- `pydanticai-process-history`: AgentPool uses PydanticAI's `ProcessHistory` capability directly for history processing +- `pydanticai-tool-thinning`: AgentPool `Tool` becomes a thin wrapper over `pydantic_ai.tools.Tool`, removing redundant metadata layers +- `pydanticai-event-passthrough`: AgentPool streaming events pass through PydanticAI's `AgentStreamEvent` types directly instead of subclassing + +### Modified Capabilities + +- `pending-message-queue`: Native agent path no longer uses `PromptInjectionManager.queue()`/`pop_queued()` for follow-up prompts — fully delegated to `PendingMessageDrainCapability`. ACP path unchanged. +- `agentnode-wrapper`: `AgentNode` uses PydanticAI's native event types directly instead of AgentPool-wrapped event subclasses. + +## Impact + +- **Code reduction**: ~1,000-1,500 lines removed (Hooks hierarchy ~400 lines, ProcessHistoryAdapter ~200 lines, PromptInjectionManager native path ~143 lines, Tool metadata simplification ~300 lines, event subclass removal ~200 lines) +- **Dependencies**: No new dependencies; reduces internal coupling to `pydantic_ai._internal` APIs +- **Breaking changes**: Hook YAML config schema changes; `ToolKind` enum removed; `PartStartEvent`/`PartDeltaEvent` no longer have `session_id` field (use context instead); `ToolResult.structured_content` removed (use PydanticAI's native structured return) +- **Affected files**: `src/agentpool/hooks/`, `src/agentpool/agents/native_agent/agent.py`, `src/agentpool/agents/native_agent/hook_manager.py`, `src/agentpool/agents/native_agent/process_history_capability.py`, `src/agentpool/agents/prompt_injection.py`, `src/agentpool/tools/base.py`, `src/agentpool/agents/events/events.py`, `src/agentpool/orchestrator/run_executor.py`, `src/agentpool/resource_providers/base.py` +- **Test impact**: Hook tests, tool tests, and event tests need updating. Behavioral parity must be verified for all existing scenarios. diff --git a/openspec/changes/pydanticai-thinning-refactor/specs/agentnode-wrapper/spec.md b/openspec/changes/pydanticai-thinning-refactor/specs/agentnode-wrapper/spec.md new file mode 100644 index 000000000..96c005a25 --- /dev/null +++ b/openspec/changes/pydanticai-thinning-refactor/specs/agentnode-wrapper/spec.md @@ -0,0 +1,43 @@ +## MODIFIED Requirements + +### Requirement: AgentNode wraps AgentPool agents as BaseNode +AgentPool SHALL provide `AgentNode` — a `pydantic_graph.BaseNode` implementation that wraps an AgentPool agent for graph execution without modifying the agent's lifecycle or `MessageNode`. + +#### Scenario: AgentNode execution creates child session +- **WHEN** `AgentNode.run()` is invoked during graph execution +- **THEN** it generates a session ID, creates a child session via `SessionPool.create_session(session_id, agent_name, parent_session_id)`, and runs the wrapped agent within that session + +#### Scenario: AgentNode preserves agent lifecycle +- **WHEN** an agent is wrapped in `AgentNode` +- **THEN** the agent's signals, connections, MCP servers, and event handlers remain functional and independent of graph execution + +#### Scenario: AgentNode handles streaming events using PydanticAI native event types +- **WHEN** an agent wrapped in `AgentNode` emits streaming events during `_run_stream_once()` +- **THEN** events are iterated and collected using PydanticAI's native `AgentStreamEvent` types (not AgentPool subclasses) +- **AND** the final `StreamCompleteEvent` provides the result message +- **AND** if no `StreamCompleteEvent` is emitted, a `RuntimeError` is raised +- **AND** `session_id` is accessed via `AgentContext`, not from event payload fields + +#### Scenario: AgentNode passes session state via context (agent is stateless) +- **WHEN** `AgentNode.run()` begins execution +- **THEN** it passes `session_id` via `AgentRunContext` and `_run_stream_once(session_id=...)` parameters; the agent instance itself is NOT mutated (no `agent.session_id` assignment) + +#### Scenario: AgentNode returns End[ChatMessage] +- **WHEN** `AgentNode.run()` completes successfully +- **THEN** it returns `End[ChatMessage]` (as required by pydantic_graph `BaseNode.run()`), wrapping the agent's output message + +#### Scenario: AgentNode avoids method name collision +- **WHEN** `AgentNode` executes the wrapped agent +- **THEN** it calls the agent's internal execution method (`_run_stream_once()`), NOT the public `agent.run()` which delegates to SessionPool and would create double session creation + +#### Scenario: AgentNode accesses graph deps via ctx.deps +- **WHEN** `AgentNode.run()` needs graph-level state (session_id, event_bus, prompt) +- **THEN** it accesses them via `ctx.deps` (type `GraphDeps`), NOT via `ctx.state` (type `ChatMessage`) + +#### Scenario: AgentNode uses ctx.state for sequential chains +- **WHEN** `AgentNode` is part of a sequential chain and `ctx.state` is available +- **THEN** it passes `ctx.state` (the previous node's output) as the agent input, NOT `ctx.deps.prompt` + +#### Scenario: AgentNode uses ctx.deps.prompt for initial input +- **WHEN** `AgentNode` is the first node in a graph and `ctx.state` is None +- **THEN** it falls back to `ctx.deps.prompt` as the agent input diff --git a/openspec/changes/pydanticai-thinning-refactor/specs/pending-message-queue/spec.md b/openspec/changes/pydanticai-thinning-refactor/specs/pending-message-queue/spec.md new file mode 100644 index 000000000..0da5ef186 --- /dev/null +++ b/openspec/changes/pydanticai-thinning-refactor/specs/pending-message-queue/spec.md @@ -0,0 +1,37 @@ +## MODIFIED Requirements + +### Requirement: PydanticAI pending message queue replaces manual follow-up prompt queue for native agents only +The system SHALL use PydanticAI's `PendingMessageDrainCapability` for follow-up prompt delivery on native agents. `RunExecutor` (native-agent turn driver) SHALL NOT maintain `_post_turn_prompts` or `_injection_locks` for follow-up prompts. `BaseAgent._run_stream_once()` SHALL NOT contain its own internal prompt continuation loop for native agents. `PromptInjectionManager.queue()`/`pop_queued()`/`flush_pending_to_queue()` SHALL NOT be called for native agents. + +**CRITICAL**: `PromptInjectionManager.inject()`/`consume()` (tool result augmentation via `after_tool_execute`) is NOT replaced by PydanticAI's queue. This mechanism modifies tool results, not conversation messages. It SHALL be preserved for native agents. + +**CRITICAL**: `PromptInjectionManager.queue()`/`pop_queued()`/`flush_pending_to_queue()` SHALL be preserved for ACP (non-native) agents that do not use PydanticAI's agent loop. + +#### Scenario: Tool enqueues steering message on native agent +- **WHEN** a tool calls `ctx.enqueue(content, priority='asap')` during a native turn +- **THEN** PydanticAI's `PendingMessageDrainCapability` drains it before the next `ModelRequest` +- **AND** the message is injected into the active conversation + +#### Scenario: External code enqueues follow-up message on native agent +- **WHEN** external code calls `pydantic_ai_run.enqueue(content, priority='when_idle')` while a native run is active +- **THEN** the message remains queued until the agent would otherwise terminate +- **AND** PydanticAI extends the run with an additional model request + +#### Scenario: No manual auto-resume needed for native agents +- **WHEN** a follow-up message is queued after a native turn ends +- **THEN** PydanticAI's `after_node_run` hook automatically drains the queue +- **AND** no `_trigger_auto_resume()` or `_process_queued_work()` logic is executed +- **AND** no `PromptInjectionManager.queue()` or `pop_queued()` is called for native agents + +#### Scenario: Tool result augmentation still works for native agents +- **WHEN** a tool calls `agent.inject_prompt("also check tests")` during a native turn +- **THEN** `PromptInjectionManager.inject()` stores the message +- **AND** the `Hooks` capability's `after_tool_execute` callback consumes it via `injection_manager.consume()` +- **AND** the injected context is added to the tool result (wrapped in `` tags) +- **AND** this is separate from PydanticAI's `enqueue()` conversation queue + +#### Scenario: ACP agent still uses manual queue +- **WHEN** a follow-up message is queued for an ACP (non-native) agent +- **THEN** `PromptInjectionManager.queue()` and `pop_queued()` are used (manual queue preserved) +- **AND** `TurnRunner._process_queued_work()` drains the queue +- **AND** `PendingMessageDrainCapability` is not involved (ACP agents don't use PydanticAI's agent loop) diff --git a/openspec/changes/pydanticai-thinning-refactor/specs/pydanticai-event-passthrough/spec.md b/openspec/changes/pydanticai-thinning-refactor/specs/pydanticai-event-passthrough/spec.md new file mode 100644 index 000000000..eb0962e21 --- /dev/null +++ b/openspec/changes/pydanticai-thinning-refactor/specs/pydanticai-event-passthrough/spec.md @@ -0,0 +1,35 @@ +## ADDED Requirements + +### Requirement: AgentPool streaming events pass through PydanticAI event types directly +AgentPool SHALL use PydanticAI's `AgentStreamEvent` types directly in streaming. The custom `PartStartEvent(PyAIPartStartEvent)` and `PartDeltaEvent(PyAIPartDeltaEvent)` subclasses SHALL be removed. `session_id` SHALL be accessed via `RunContext.deps` (which carries `AgentContext` containing `session_id`), not from event payload fields. + +#### Scenario: Streaming event is a raw PydanticAI event type +- **WHEN** a native agent emits a `PartStartEvent` during streaming +- **THEN** the event is a `pydantic_ai.AgentStreamEvent` instance, not an AgentPool subclass +- **AND** the event does not have a `session_id` field +- **AND** consumers access `session_id` via `run_ctx.session_id` or `AgentContext.session_id` + +#### Scenario: RunExecutor forwards PydanticAI events without wrapping +- **WHEN** `RunExecutor.execute()` receives a `PartStartEvent` or `PartDeltaEvent` from PydanticAI's `agent_run.next(node)` +- **THEN** it forwards the event as-is to the event queue +- **AND** no subclassing or field-addition wrapping occurs + +### Requirement: ToolCallStartEvent and ToolCallCompleteEvent are thin wrappers +`ToolCallStartEvent` and `ToolCallCompleteEvent` SHALL be constructed by `RunExecutor` as thin dataclass instances from PydanticAI's `FunctionToolCallEvent` and `FunctionToolResultEvent`, not by subclassing PydanticAI event types. They SHALL carry only AgentPool-specific fields not present on PydanticAI's events. + +#### Scenario: ToolCallStartEvent constructed from FunctionToolCallEvent +- **WHEN** `RunExecutor` receives a `FunctionToolCallEvent` from PydanticAI +- **THEN** it constructs a `ToolCallStartEvent` with `tool_name`, `tool_call_id`, and `raw_input` extracted from the PydanticAI event +- **AND** the `ToolCallStartEvent` does not subclass any PydanticAI event type +- **AND** the event is published to the EventBus + +#### Scenario: ToolCallCompleteEvent constructed from FunctionToolResultEvent +- **WHEN** `RunExecutor` receives a `FunctionToolResultEvent` from PydanticAI +- **THEN** it constructs a `ToolCallCompleteEvent` with `tool_name`, `tool_call_id`, `tool_result`, and metadata extracted from the PydanticAI event +- **AND** the `ToolCallCompleteEvent` does not subclass any PydanticAI event type + +## REMOVED Requirements + +### Requirement: PartStartEvent and PartDeltaEvent subclass PydanticAI events with session_id +**Reason**: Subclassing PydanticAI events just to add `session_id` creates coupling — every PydanticAI event change requires AgentPool to update subclasses. `session_id` is already available via `AgentContext` in `RunContext.deps`. Protocol consumers can access it from context, not event payload. +**Migration**: Replace `event.session_id` access with `run_ctx.session_id` or `AgentContext.session_id` lookups. diff --git a/openspec/changes/pydanticai-thinning-refactor/specs/pydanticai-hooks-delegation/spec.md b/openspec/changes/pydanticai-thinning-refactor/specs/pydanticai-hooks-delegation/spec.md new file mode 100644 index 000000000..482392b1c --- /dev/null +++ b/openspec/changes/pydanticai-thinning-refactor/specs/pydanticai-hooks-delegation/spec.md @@ -0,0 +1,55 @@ +## ADDED Requirements + +### Requirement: AgentPool delegates hook lifecycle to PydanticAI Hooks capability +AgentPool SHALL use `pydantic_ai.capabilities.Hooks` as the primary hook lifecycle mechanism for native agents. `NativeAgentHookManager.as_capability()` SHALL return a `pydantic_ai.capabilities.Hooks` instance with typed callbacks (`before_run`/`after_run`/`before_tool_execute`/`after_tool_execute`). The custom `Hook` base class, `CallableHook`, regex matchers, timeout handling, and parallel result combining logic SHALL be removed. + +#### Scenario: Native agent uses PydanticAI Hooks capability +- **WHEN** a native agent is created via `get_agentlet()` +- **THEN** the agent's capabilities list includes a `pydantic_ai.capabilities.Hooks` instance +- **AND** the `Hooks` instance has callbacks registered for `before_tool_execute` and `after_tool_execute` +- **AND** no custom `Hook` base class or `CallableHook` instances are present in the capability pipeline + +#### Scenario: Multiple hooks combined with priority semantics +- **WHEN** multiple hooks are registered (e.g., two `before_tool_execute` hooks, one returning "deny" and one returning "allow") +- **THEN** the `Hooks` callback wrapper combines results using priority: deny > ask > allow +- **AND** the combined result is returned to PydanticAI as the single callback result + +#### Scenario: Hook fires at correct lifecycle point +- **WHEN** a tool is about to execute during a native agent run +- **THEN** the `before_tool_execute` callback on the `Hooks` capability fires +- **AND** the callback receives `RunContext` and tool call details +- **AND** the callback can return a decision (allow/deny/ask) that PydanticAI respects + +### Requirement: CommandHook and PromptHook survive as thin adapters over Hooks callbacks +AgentPool SHALL preserve `CommandHook` (subprocess evaluation) and `PromptHook` (LLM evaluation) as thin adapter classes that register their logic as `pydantic_ai.capabilities.Hooks` callbacks. They SHALL NOT inherit from a custom `Hook` base class. They SHALL implement their evaluation logic inside `Hooks` callback functions. + +#### Scenario: CommandHook registers as Hooks callback +- **WHEN** a `CommandHook` is configured for an agent +- **THEN** it registers its subprocess evaluation logic as a `before_tool_execute` callback on the `Hooks` capability +- **AND** the subprocess is spawned with the tool call details as input +- **AND** the subprocess stdout is parsed as the hook result (allow/deny/ask) + +#### Scenario: PromptHook registers as Hooks callback +- **WHEN** a `PromptHook` is configured for an agent +- **THEN** it registers its LLM evaluation logic as a `before_tool_execute` callback on the `Hooks` capability +- **AND** a mini PydanticAI `Agent` evaluates the tool call against the prompt template +- **AND** the LLM response is parsed as the hook result (allow/deny/ask) + +### Requirement: Hook YAML config uses callback references instead of matcher/event/timeout +AgentPool hook YAML configuration SHALL use callback references (`before_run`/`after_run`/`before_tool_execute`/`after_tool_execute`) instead of `matcher`/`event`/`timeout` fields. A deprecation shim SHALL auto-translate old-format configs (`matcher`/`event`/`timeout`) to the new format during a transition period. + +#### Scenario: New-format hook config +- **WHEN** a YAML config defines a hook with `before_tool_execute: "mymodule:check_tool"` +- **THEN** the hook is registered as a `before_tool_execute` callback on the `Hooks` capability +- **AND** no `matcher`/`event`/`timeout` fields are present + +#### Scenario: Old-format hook config with deprecation shim +- **WHEN** a YAML config defines a hook with `event: "before_tool_execute"` and `matcher: "bash.*"` +- **THEN** the deprecation shim translates it to `before_tool_execute` callback with a wrapper that applies the regex matcher internally +- **AND** a `DeprecationWarning` is emitted with migration guidance + +## REMOVED Requirements + +### Requirement: Custom Hook base class with regex matchers and timeout handling +**Reason**: PydanticAI's `Hooks` capability provides the same lifecycle hooks with a cleaner API. Regex matchers, timeout handling, and parallel result combining are implementation details that belong inside callback wrappers, not in a base class hierarchy. +**Migration**: Replace `Hook` subclasses with `pydantic_ai.capabilities.Hooks` callbacks. Move regex matching and timeout logic inside the callback wrapper function. diff --git a/openspec/changes/pydanticai-thinning-refactor/specs/pydanticai-process-history/spec.md b/openspec/changes/pydanticai-thinning-refactor/specs/pydanticai-process-history/spec.md new file mode 100644 index 000000000..039e1c656 --- /dev/null +++ b/openspec/changes/pydanticai-thinning-refactor/specs/pydanticai-process-history/spec.md @@ -0,0 +1,27 @@ +## ADDED Requirements + +### Requirement: AgentPool uses PydanticAI ProcessHistory capability directly +AgentPool SHALL use `pydantic_ai.capabilities.ProcessHistory` for history processing on native agents. The custom `ProcessHistoryAdapter` class SHALL be removed. Custom history processors (compaction, etc.) SHALL be registered as callbacks on PydanticAI's `ProcessHistory` capability. + +#### Scenario: Native agent uses PydanticAI ProcessHistory +- **WHEN** a native agent is created via `get_agentlet()` with history processors configured +- **THEN** the agent's capabilities list includes a `pydantic_ai.capabilities.ProcessHistory` instance +- **AND** custom history processors (compaction, etc.) are registered as callbacks on the `ProcessHistory` capability +- **AND** no `ProcessHistoryAdapter` instance is present in the capability pipeline + +#### Scenario: History processor callback fires at correct time +- **WHEN** a native agent is about to make a model request with existing message history +- **THEN** the `ProcessHistory` callback fires with the current message history +- **AND** the callback can modify the history (e.g., compact old messages) +- **AND** the modified history is used for the model request + +#### Scenario: Multiple history processors execute in order +- **WHEN** multiple history processors are configured (e.g., compaction + token limit trimming) +- **THEN** they execute in the order they were registered as `ProcessHistory` callbacks +- **AND** each processor receives the output of the previous processor + +## REMOVED Requirements + +### Requirement: Custom ProcessHistoryAdapter with caching and signature validation +**Reason**: PydanticAI's `ProcessHistory` capability provides the same functionality with a simpler API. The caching layer in `ProcessHistoryAdapter` was needed because AgentPool rebuilt the agent per-run, but PydanticAI's `ProcessHistory` already handles lifecycle efficiently. Signature validation was for detecting config changes — now handled by the capability rebuild mechanism in `get_agentlet()`. +**Migration**: Register history processors directly as callbacks on `pydantic_ai.capabilities.ProcessHistory` instead of wrapping them in `ProcessHistoryAdapter`. diff --git a/openspec/changes/pydanticai-thinning-refactor/specs/pydanticai-tool-thinning/spec.md b/openspec/changes/pydanticai-thinning-refactor/specs/pydanticai-tool-thinning/spec.md new file mode 100644 index 000000000..f7c7c846a --- /dev/null +++ b/openspec/changes/pydanticai-thinning-refactor/specs/pydanticai-tool-thinning/spec.md @@ -0,0 +1,51 @@ +## ADDED Requirements + +### Requirement: AgentPool Tool is a thin wrapper over pydantic_ai.tools.Tool +AgentPool's `Tool` dataclass SHALL be a thin wrapper over `pydantic_ai.tools.Tool`. The `Tool.to_pydantic_ai()` method SHALL produce a 1:1 mapping without complex conversion logic. Redundant metadata fields that PydanticAI's `Tool` already provides SHALL be removed. + +#### Scenario: Tool converts to PydanticAI Tool with 1:1 mapping +- **WHEN** `Tool.to_pydantic_ai()` is called on an AgentPool `Tool` instance +- **THEN** it produces a `pydantic_ai.tools.Tool` with function, name, description, and parameters mapped directly +- **AND** no complex conversion logic or edge-case handling is present in the method +- **AND** the method body is fewer than 20 lines + +#### Scenario: Tool uses PydanticAI requires_approval natively +- **WHEN** a tool is configured with `requires_confirmation: true` and no deferred execution +- **THEN** the resulting `pydantic_ai.tools.Tool` has `requires_approval=True` set directly +- **AND** no `ApprovalRequiredToolset` wrapper is applied for non-deferred confirmation + +#### Scenario: Deferred tools still use ApprovalRequiredToolset +- **WHEN** a tool is configured with `deferred: true` and a deferred strategy (`block`/`continue`/`stream`) +- **THEN** the tool uses `ApprovalRequiredToolset` wrapping (preserved for deferred execution scenarios) +- **AND** PydanticAI's `requires_approval` is not used for deferred tools + +### Requirement: ToolKind taxonomy is removed +The `ToolKind` enum and associated taxonomy (`read`/`edit`/`delete`/`execute`/etc.) SHALL be removed. Tool categorization for config validation SHALL use string-based tool name patterns instead of a formal taxonomy. + +#### Scenario: Config validation without ToolKind +- **WHEN** a YAML config defines tool permissions (e.g., `allowed_tools: ["bash", "read"]`) +- **THEN** validation uses string matching on tool names, not `ToolKind` enum values +- **AND** no `ToolKind` import or reference exists in the codebase + +#### Scenario: Tool instance has no kind field +- **WHEN** a `Tool` instance is created +- **THEN** it does not have a `kind` field or `ToolKind` attribute +- **AND** tool metadata does not include kind categorization + +### Requirement: ToolResult structured_content is removed +`ToolResult.structured_content` field SHALL be removed. Structured tool returns SHALL use PydanticAI's native `ToolReturn` structured return mechanism directly. + +#### Scenario: Tool returns structured data +- **WHEN** a tool returns structured data (e.g., a Pydantic model) +- **THEN** the tool returns a `pydantic_ai.messages.ToolReturn` with structured content set via PydanticAI's native mechanism +- **AND** no `ToolResult.structured_content` field is present on AgentPool's `ToolResult` + +## REMOVED Requirements + +### Requirement: ToolKind taxonomy for tool categorization +**Reason**: `ToolKind` was a categorization system for tool permissions, but it's only used in config validation, not at runtime. String-based tool name patterns provide the same validation capability with less overhead. +**Migration**: Replace `kind: read` config entries with tool name patterns (e.g., `allowed_tools: ["read", "grep"]`). + +### Requirement: ToolResult.structured_content for machine-readable returns +**Reason**: PydanticAI's `ToolReturn` already supports structured returns natively. Maintaining a separate `structured_content` field on AgentPool's `ToolResult` is redundant. +**Migration**: Use `pydantic_ai.messages.ToolReturn` with structured content directly. diff --git a/openspec/changes/pydanticai-thinning-refactor/tasks.md b/openspec/changes/pydanticai-thinning-refactor/tasks.md new file mode 100644 index 000000000..f4fab5b42 --- /dev/null +++ b/openspec/changes/pydanticai-thinning-refactor/tasks.md @@ -0,0 +1,72 @@ +## 1. Pre-Migration: Regression Test Baseline + +- [x] 1.1 Write regression tests for all existing hook combination scenarios (deny > ask > allow priority, parallel hooks, timeout behavior) in `tests/hooks/test_hook_combining.py` +- [x] 1.2 Write regression tests for `ProcessHistoryAdapter` behavior (caching, signature validation, processor ordering) in `tests/agents/test_process_history.py` +- [x] 1.3 Write regression tests for `PromptInjectionManager` native path (inject/consume, queue/pop_queued, flush_pending_to_queue) in `tests/agents/test_prompt_injection.py` +- [x] 1.4 Write regression tests for `Tool.to_pydantic_ai()` conversion (schema overrides, deferred metadata, approval wrapping) in `tests/tools/test_tool_conversion.py` +- [x] 1.5 Write regression tests for event subclass behavior (PartStartEvent/PartDeltaEvent with session_id, ToolCallStartEvent/ToolCallCompleteEvent) in `tests/agents/test_events.py` +- [x] 1.6 Run full test suite to establish green baseline: `uv run pytest -m unit` + +## 2. Phase 1: Hooks Migration + +- [x] 2.1 Implement `HooksCapabilityAdapter` that wraps multiple AgentPool hooks into a single `pydantic_ai.capabilities.Hooks` instance in `src/agentpool/agents/native_agent/hooks_capability_adapter.py` +- [x] 2.2 Implement priority combining logic (deny > ask > allow) inside `HooksCapabilityAdapter` callbacks, preserving exact semantics of current parallel hook combining +- [x] 2.3 Migrate `CallableHook` to register its logic as a `Hooks` callback (no inheritance from custom `Hook` base class) +- [x] 2.4 Migrate `CommandHook` as a thin adapter that spawns subprocesses inside `Hooks.before_tool_execute` callback +- [x] 2.5 Migrate `PromptHook` as a thin adapter that runs LLM evaluation inside `Hooks.before_tool_execute` callback +- [x] 2.6 Update `NativeAgentHookManager.as_capability()` to return `HooksCapabilityAdapter` instead of wrapping `AgentHooks` +- [x] 2.7 Remove `Hook` base class, `CallableHook` class hierarchy, regex matchers, timeout handling from `src/agentpool/hooks/` +- [x] 2.8 Add deprecation shim for old-format YAML hook config (`matcher`/`event`/`timeout` → callback references) with `DeprecationWarning` +- [x] 2.9 Update `agentpool_config` hook config models to use callback references (`before_run`/`after_run`/`before_tool_execute`/`after_tool_execute`) +- [x] 2.10 Run hook regression tests and verify all pass: `uv run pytest tests/hooks/ -vv` +- [x] 2.11 Run full test suite: `uv run pytest -m unit` + +## 3. Phase 2: ProcessHistory + PromptInjectionManager + +- [x] 3.1 Replace `ProcessHistoryAdapter` with direct `pydantic_ai.capabilities.ProcessHistory` capability in `get_agentlet()` assembly +- [x] 3.2 Register custom history processors (compaction, token trimming) as callbacks on `ProcessHistory` capability +- [x] 3.3 Remove `ProcessHistoryAdapter` class and its caching/signature validation logic from `src/agentpool/agents/native_agent/process_history_capability.py` +- [x] 3.4 Remove `PromptInjectionManager.queue()`/`pop_queued()`/`flush_pending_to_queue()` calls from native agent path in `base_agent.py` and `TurnRunner` +- [x] 3.5 Keep `PromptInjectionManager.inject()`/`consume()` for tool result augmentation (update `Hooks` `after_tool_execute` callback to call `consume()`) +- [x] 3.6 Keep `PromptInjectionManager.queue()`/`pop_queued()`/`flush_pending_to_queue()` for ACP agent path only +- [x] 3.7 Remove native-agent-specific `_post_turn_prompts` and `_injection_locks` from `RunExecutor` if present +- [x] 3.8 Remove native-agent follow-up loop from `BaseAgent._run_stream_once()` (the `while has_queued()` branch for native agents) +- [x] 3.9 Run ProcessHistory and PromptInjection regression tests: `uv run pytest tests/agents/test_process_history.py tests/agents/test_prompt_injection.py -vv` +- [x] 3.10 Run full test suite: `uv run pytest -m unit` + +## 4. Phase 3: Tool Thinning + +- [x] 4.1 Remove `ToolKind` enum and all references from `src/agentpool/tools/base.py` +- [x] 4.2 Replace `ToolKind`-based config validation with string-based tool name patterns in `agentpool_config` +- [x] 4.3 Remove `ToolResult.structured_content` field, update tool implementations to use PydanticAI's `ToolReturn` natively +- [x] 4.4 Simplify `Tool.to_pydantic_ai()` to direct 1:1 mapping (target: <20 lines) +- [x] 4.5 Use PydanticAI's `requires_approval=True` directly for non-deferred confirmation tools +- [x] 4.6 Keep `ApprovalRequiredToolset` wrapping only for deferred execution tools +- [x] 4.7 Remove redundant metadata fields from `Tool` dataclass that `pydantic_ai.tools.Tool` already provides +- [x] 4.8 Run tool regression tests: `uv run pytest tests/tools/ -vv` +- [x] 4.9 Run full test suite: `uv run pytest -m unit` + +## 5. Phase 3: Event Passthrough + +- [x] 5.1 Remove `PartStartEvent(PyAIPartStartEvent)` and `PartDeltaEvent(PyAIPartDeltaEvent)` subclasses from `src/agentpool/agents/events/events.py` +- [x] 5.2 Audit all `event.session_id` access points and replace with `run_ctx.session_id` or `AgentContext.session_id` lookups +- [x] 5.3 Update `RunExecutor` to forward PydanticAI `PartStartEvent`/`PartDeltaEvent` as-is without wrapping +- [x] 5.4 Convert `ToolCallStartEvent` and `ToolCallCompleteEvent` from PydanticAI event subclasses to plain dataclass instances constructed by `RunExecutor` +- [x] 5.5 Update `EventBus` event routing to handle plain PydanticAI event types +- [x] 5.6 Update protocol server event consumers (`ProtocolEventConsumerMixin` implementations) to get `session_id` from context, not event payload +- [x] 5.7 Update `RichAgentStreamEvent` union type to include raw `AgentStreamEvent` instead of AgentPool subclasses +- [x] 5.8 Run event regression tests: `uv run pytest tests/agents/test_events.py -vv` +- [x] 5.9 Run full test suite: `uv run pytest -m unit` + +## 6. Post-Migration: Cleanup & Verification + +- [x] 6.1 Run `uv run ruff check src/` and fix all lint errors +- [x] 6.2 Run `uv run ruff format --check src/` and format if needed +- [x] 6.3 Run `uv run --no-group docs mypy src/` and fix all type errors +- [x] 6.4 Run full test suite with coverage: `uv run pytest --cov-report=term-missing` +- [x] 6.5 Verify no `pydantic_ai._internal` or `pydantic_ai._function_schema` imports remain in non-bridge code +- [x] 6.6 Update `AGENTS.md` documentation to reflect new architecture (Hooks delegation, ProcessHistory direct usage, event passthrough) +- [x] 6.7 Update CHANGELOG with breaking changes (Hook config schema, ToolKind removal, event subclass removal) +- [x] 6.8 Write migration guide for YAML config changes in `docs/migration/pydanticai-thinning.md` +- [x] 6.9 Run integration tests: `uv run pytest -m integration` +- [x] 6.10 Final full suite run: `uv run pytest` diff --git a/src/agentpool/agents/native_agent/hooks_capability_adapter.py b/src/agentpool/agents/native_agent/hooks_capability_adapter.py new file mode 100644 index 000000000..7798c6820 --- /dev/null +++ b/src/agentpool/agents/native_agent/hooks_capability_adapter.py @@ -0,0 +1,403 @@ +"""Adapter bridging AgentPool hooks to pydantic_ai.capabilities.Hooks. + +Replaces the deprecated ``AgentHooks.as_capability()`` bridge with a +focused adapter that: + +- Accepts hook callables organized by pydantic-ai callback name +- Implements deny > ask > allow priority combining inside callbacks +- Wraps CommandHook and PromptHook as thin callback adapters +- Produces a single ``pydantic_ai.capabilities.Hooks`` instance +""" + +from __future__ import annotations + +import asyncio +import re +from typing import TYPE_CHECKING, Any, Literal, TypeAlias, cast + +from pydantic_ai.capabilities import Hooks + +from agentpool.hooks.base import HookInput, HookResult +from agentpool.log import get_logger + + +if TYPE_CHECKING: + from collections.abc import Awaitable, Callable, Sequence + + from exxec import ExecutionEnvironment + from pydantic_ai import AgentRunResult + from pydantic_ai.capabilities.abstract import ValidatedToolArgs + from pydantic_ai.messages import ToolCallPart + from pydantic_ai.tools import RunContext, ToolDefinition + + +logger = get_logger(__name__) + +HookCallable: TypeAlias = "Callable[..., HookResult | None | Awaitable[HookResult | None]]" # noqa: UP040 + +HookEventName: TypeAlias = Literal[ # noqa: UP040 + "before_run", "after_run", "before_tool_execute", "after_tool_execute" +] + +class HooksCapabilityAdapter: + """Builds a ``pydantic_ai.capabilities.Hooks`` from AgentPool hook callables. + + Each hook callable receives keyword arguments matching ``HookInput`` + fields and returns a ``HookResult`` (or ``None`` for implicit allow). + + The adapter handles: + - Parallel execution of multiple hooks per callback + - Priority combining: deny > ask > allow + - Matcher filtering (regex on tool_name + input_match on tool_input fields) + - Deny → ``RuntimeError`` (since pydantic-ai hooks can't return "block") + - modified_input merging into validated tool args + """ + + def __init__( + self, + *, + before_run: Sequence[HookCallable] | None = None, + after_run: Sequence[HookCallable] | None = None, + before_tool_execute: Sequence[HookCallable] | None = None, + after_tool_execute: Sequence[HookCallable] | None = None, + matchers: dict[HookEventName, Sequence[str | None]] | None = None, + input_matchers: dict[HookEventName, Sequence[dict[str, str] | None]] | None = None, + ) -> None: + self._before_run = list(before_run) if before_run else [] + self._after_run = list(after_run) if after_run else [] + self._before_tool = list(before_tool_execute) if before_tool_execute else [] + self._after_tool = list(after_tool_execute) if after_tool_execute else [] + self._matchers = matchers or {} + self._input_matchers = input_matchers or {} + + def build(self) -> Hooks: + """Build and return a ``pydantic_ai.capabilities.Hooks`` instance.""" + kwargs: dict[str, Any] = {} + + if self._before_run: + kwargs["before_run"] = self._wrap_before_run() + if self._after_run: + kwargs["after_run"] = self._wrap_after_run() + if self._before_tool: + kwargs["before_tool_execute"] = self._wrap_before_tool_execute() + if self._after_tool: + kwargs["after_tool_execute"] = self._wrap_after_tool_execute() + + return Hooks(**kwargs) + + # ----------------------------------------------------------------------- + # Priority combining — the core algorithm + # ----------------------------------------------------------------------- + + @staticmethod + async def _combine( + hooks: Sequence[HookCallable], + input_data: HookInput, + event: HookEventName, + matchers: Sequence[str | None] | None = None, + input_matchers: Sequence[dict[str, str] | None] | None = None, + env: ExecutionEnvironment | None = None, + ) -> HookResult: + if not hooks: + return HookResult(decision="allow") + + matching = [ + (i, h) + for i, h in enumerate(hooks) + if _matches( + input_data, + event, + matchers[i] if matchers else None, + input_matchers[i] if input_matchers else None, + ) + ] + if not matching: + return HookResult(decision="allow") + + raw_results = await asyncio.gather( + *(_execute_hook(h, input_data, env=env) for _, h in matching), + return_exceptions=True, + ) + + combined = HookResult(decision="allow") + reasons: list[str] = [] + contexts: list[str] = [] + + for raw_result in raw_results: + if isinstance(raw_result, BaseException): + logger.warning( + "Hook execution failed", + error=str(raw_result), + error_type=type(raw_result).__name__, + hook_event=event, + ) + continue + + result = _normalize_result(raw_result) + + if result.get("decision") == "deny": + combined["decision"] = "deny" + elif result.get("decision") == "ask" and combined.get("decision") != "deny": + combined["decision"] = "ask" + + if reason := result.get("reason"): + reasons.append(reason) + + if modified := result.get("modified_input"): + if "modified_input" not in combined: + combined["modified_input"] = {} + combined["modified_input"].update(modified) + + if "modified_output" in result: + combined["modified_output"] = result["modified_output"] + + if ctx := result.get("additional_context"): + contexts.append(ctx) + + if result.get("continue_") is False: + combined["continue_"] = False + + if reasons: + combined["reason"] = "; ".join(reasons) + if contexts: + combined["additional_context"] = "\n".join(contexts) + + return combined + + # ----------------------------------------------------------------------- + # Callback wrappers + # ----------------------------------------------------------------------- + + def _wrap_before_run(self) -> Callable[..., Awaitable[None]]: + async def wrapped(ctx: RunContext[Any]) -> None: + agent_ctx = ctx.deps + input_data = HookInput( + event="pre_run", + agent_name=_get_agent_name(agent_ctx), + session_id=_get_session_id(agent_ctx), + ) + result = await self._combine( + self._before_run, + input_data, + "before_run", + matchers=self._matchers.get("before_run"), + input_matchers=self._input_matchers.get("before_run"), + ) + if result.get("decision") == "deny": + msg = f"Run blocked: {result.get('reason', 'pre_run hook denied')}" + raise RuntimeError(msg) + + return wrapped + + def _wrap_after_run(self) -> Callable[..., Awaitable[AgentRunResult[Any]]]: + async def wrapped( + ctx: RunContext[Any], *, result: AgentRunResult[Any] + ) -> AgentRunResult[Any]: + agent_ctx = ctx.deps + input_data = HookInput( + event="post_run", + agent_name=_get_agent_name(agent_ctx), + result=result, + session_id=_get_session_id(agent_ctx), + ) + await self._combine( + self._after_run, + input_data, + "after_run", + matchers=self._matchers.get("after_run"), + input_matchers=self._input_matchers.get("after_run"), + ) + return result + + return wrapped + + def _wrap_before_tool_execute(self) -> Callable[..., Awaitable[ValidatedToolArgs]]: + async def wrapped( + ctx: RunContext[Any], + *, + call: ToolCallPart, + tool_def: ToolDefinition, + args: ValidatedToolArgs, + ) -> ValidatedToolArgs: + agent_ctx = ctx.deps + input_data = HookInput( + event="pre_tool_use", + agent_name=_get_agent_name(agent_ctx), + tool_name=call.tool_name, + tool_input=dict(args), + session_id=_get_session_id(agent_ctx), + ) + result = await self._combine( + self._before_tool, + input_data, + "before_tool_execute", + matchers=self._matchers.get("before_tool_execute"), + input_matchers=self._input_matchers.get("before_tool_execute"), + ) + if result.get("decision") == "deny": + msg = f"Tool execution blocked: {result.get('reason', 'pre_tool_use hook denied')}" + raise RuntimeError(msg) + if modified := result.get("modified_input"): + return {**dict(args), **modified} + return args + + return wrapped + + def _wrap_after_tool_execute(self) -> Callable[..., Awaitable[Any]]: + async def wrapped( + ctx: RunContext[Any], + *, + call: ToolCallPart, + tool_def: ToolDefinition, + args: ValidatedToolArgs, + result: Any, + ) -> Any: + agent_ctx = ctx.deps + input_data = HookInput( + event="post_tool_use", + agent_name=_get_agent_name(agent_ctx), + tool_name=call.tool_name, + tool_input=dict(args), + tool_output=result, + duration_ms=0.0, + session_id=_get_session_id(agent_ctx), + ) + await self._combine( + self._after_tool, + input_data, + "after_tool_execute", + matchers=self._matchers.get("after_tool_execute"), + input_matchers=self._input_matchers.get("after_tool_execute"), + ) + return result + + return wrapped + + +# --------------------------------------------------------------------------- +# Module-level helpers +# --------------------------------------------------------------------------- + + +def _get_agent_name(agent_ctx: Any) -> str: + return agent_ctx.node_name if agent_ctx else "" + + +def _get_session_id(agent_ctx: Any) -> str | None: + if agent_ctx and agent_ctx.run_ctx: + session_id: str | None = agent_ctx.run_ctx.session_id + return session_id + return None + + +def _matches( + input_data: HookInput, + event: HookEventName, + matcher: str | None, + input_match: dict[str, str] | None, +) -> bool: + if matcher and matcher != "*": + pattern = re.compile(matcher) + if event in ("before_tool_execute", "after_tool_execute"): + tool_name = input_data.get("tool_name", "") + if not pattern.search(tool_name): + return False + + if input_match: + tool_input = input_data.get("tool_input") or {} + for key, pat_str in input_match.items(): + pat = re.compile(pat_str) + value = str(tool_input.get(key, "")) + if not pat.search(value): + return False + + return True + + +async def _execute_hook( + hook: HookCallable, + input_data: HookInput, + *, + env: ExecutionEnvironment | None = None, +) -> HookResult | None: + import inspect + + kwargs = dict(input_data) + if env is not None: + kwargs["env"] = env + + if inspect.iscoroutinefunction(hook): + result = await hook(**kwargs) + else: + result = await asyncio.get_event_loop().run_in_executor(None, lambda: hook(**kwargs)) + return _normalize_result(result) + + +def _normalize_result(raw: Any) -> HookResult: + if raw is None: + return HookResult(decision="allow") + if isinstance(raw, dict): + return cast("HookResult", raw) + if isinstance(raw, str): + return HookResult(decision="allow", additional_context=raw) + if isinstance(raw, bool): + return HookResult(decision="allow" if raw else "deny") + return HookResult(decision="allow") + + +def from_agent_hooks(agent_hooks: Any) -> HooksCapabilityAdapter: + """Create a ``HooksCapabilityAdapter`` from a legacy ``AgentHooks`` instance. + + Extracts hook callables and matcher metadata from ``AgentHooks`` + (which holds ``Hook`` instances with ``fn``, ``matcher``, ``input_match`` + attributes) and constructs an adapter with equivalent behavior. + """ + + def _extract( + hooks: Sequence[Any], + ) -> tuple[ + list[HookCallable], + list[str | None], + list[dict[str, str] | None], + ]: + callables: list[HookCallable] = [] + matchers: list[str | None] = [] + input_matchers: list[dict[str, str] | None] = [] + for h in hooks: + if hasattr(h, "fn"): + callables.append(h.fn) + elif callable(h): + callables.append(h) + matchers.append(getattr(h, "matcher", None)) + input_matchers.append(getattr(h, "input_match", None)) + return callables, matchers, input_matchers + + pre_run_c, pre_run_m, pre_run_im = _extract(agent_hooks.pre_run) + post_run_c, post_run_m, post_run_im = _extract(agent_hooks.post_run) + pre_tool_c, pre_tool_m, pre_tool_im = _extract(agent_hooks.pre_tool_use) + post_tool_c, post_tool_m, post_tool_im = _extract(agent_hooks.post_tool_use) + + matchers: dict[HookEventName, Sequence[str | None]] = {} + input_matchers: dict[HookEventName, Sequence[dict[str, str] | None]] = {} + + if pre_run_c: + matchers["before_run"] = pre_run_m + input_matchers["before_run"] = pre_run_im + if post_run_c: + matchers["after_run"] = post_run_m + input_matchers["after_run"] = post_run_im + if pre_tool_c: + matchers["before_tool_execute"] = pre_tool_m + input_matchers["before_tool_execute"] = pre_tool_im + if post_tool_c: + matchers["after_tool_execute"] = post_tool_m + input_matchers["after_tool_execute"] = post_tool_im + + return HooksCapabilityAdapter( + before_run=pre_run_c or None, + after_run=post_run_c or None, + before_tool_execute=pre_tool_c or None, + after_tool_execute=post_tool_c or None, + matchers=matchers or None, + input_matchers=input_matchers or None, + ) diff --git a/tests/agents/test_events.py b/tests/agents/test_events.py new file mode 100644 index 000000000..ede74877f --- /dev/null +++ b/tests/agents/test_events.py @@ -0,0 +1,312 @@ +"""Regression tests for AgentPool event types. + +Tests cover: +- ``PartStartEvent`` / ``PartDeltaEvent`` subclassing behavior (session_id field) +- ``ToolCallStartEvent`` / ``ToolCallCompleteEvent`` construction +- ``RunStartedEvent`` / ``StreamCompleteEvent`` fields +- ``RichAgentStreamEvent`` union membership + +These serve as a **behavioral baseline** before the thinning refactor +removes the PydanticAI event subclasses. +""" + +from __future__ import annotations + +from typing import Any + +import pytest +from pydantic_ai import PartStartEvent as PyAIPartStartEvent +from pydantic_ai import PartDeltaEvent as PyAIPartDeltaEvent +from pydantic_ai.messages import TextPart, ThinkingPart, TextPartDelta, ThinkingPartDelta + +from agentpool.agents.events.events import ( + PartDeltaEvent, + PartStartEvent, + RichAgentStreamEvent, + RunFailedEvent, + RunStartedEvent, + StreamCompleteEvent, + ToolCallCompleteEvent, + ToolCallStartEvent, +) + + +# --------------------------------------------------------------------------- +# PartStartEvent — subclass behavior +# --------------------------------------------------------------------------- + + +def test_part_start_event_is_pydantic_ai_subclass(): + """PartStartEvent subclasses pydantic_ai.PartStartEvent.""" + assert issubclass(PartStartEvent, PyAIPartStartEvent) + + +def test_part_start_event_has_session_id(): + """PartStartEvent has a session_id field defaulting to empty string.""" + event = PartStartEvent(index=0, part=TextPart(content="hello")) + assert hasattr(event, "session_id") + assert event.session_id == "" + + +def test_part_start_event_session_id_default(): + """PartStartEvent session_id defaults to empty string.""" + event = PartStartEvent(index=0, part=TextPart(content="hi")) + assert hasattr(event, "session_id") + assert event.session_id == "" + + +def test_part_start_event_thinking_factory(): + """PartStartEvent.thinking() creates a thinking part event.""" + event = PartStartEvent.thinking(index=0, content="reasoning...") + assert isinstance(event, PartStartEvent) + assert isinstance(event.part, ThinkingPart) + + +def test_part_start_event_text_factory(): + """PartStartEvent.text() creates a text part event.""" + event = PartStartEvent.text(index=0, content="response") + assert isinstance(event, PartStartEvent) + assert isinstance(event.part, TextPart) + + +# --------------------------------------------------------------------------- +# PartDeltaEvent — subclass behavior +# --------------------------------------------------------------------------- + + +def test_part_delta_event_is_pydantic_ai_subclass(): + """PartDeltaEvent subclasses pydantic_ai.PartDeltaEvent.""" + assert issubclass(PartDeltaEvent, PyAIPartDeltaEvent) + + +def test_part_delta_event_has_session_id(): + """PartDeltaEvent has a session_id field defaulting to empty string.""" + event = PartDeltaEvent(index=0, delta=TextPartDelta(content_delta="chunk")) + assert hasattr(event, "session_id") + assert event.session_id == "" + + +def test_part_delta_event_text_factory(): + """PartDeltaEvent.text() creates a text delta event.""" + event = PartDeltaEvent.text(index=0, content="chunk") + assert isinstance(event, PartDeltaEvent) + assert isinstance(event.delta, TextPartDelta) + + +def test_part_delta_event_thinking_factory(): + """PartDeltaEvent.thinking() creates a thinking delta event.""" + event = PartDeltaEvent.thinking(index=0, content="reason") + assert isinstance(event, PartDeltaEvent) + assert isinstance(event.delta, ThinkingPartDelta) + + +def test_part_delta_event_tool_call_factory(): + """PartDeltaEvent.tool_call() creates a tool call delta event.""" + event = PartDeltaEvent.tool_call(index=0, content='{"x":1}', tool_call_id="tc-1") + assert isinstance(event, PartDeltaEvent) + + +# --------------------------------------------------------------------------- +# RunStartedEvent +# --------------------------------------------------------------------------- + + +def test_run_started_event_fields(): + """RunStartedEvent has the expected fields.""" + event = RunStartedEvent( + run_id="run-1", + agent_name="my_agent", + session_id="sess-1", + parent_session_id=None, + ) + assert event.run_id == "run-1" + assert event.agent_name == "my_agent" + assert event.session_id == "sess-1" + assert event.parent_session_id is None + assert event.event_kind == "run_started" + + +def test_run_started_event_with_parent(): + """RunStartedEvent with parent_session_id.""" + event = RunStartedEvent( + run_id="run-2", + agent_name="child", + session_id="child-sess", + parent_session_id="parent-sess", + ) + assert event.parent_session_id == "parent-sess" + + +def test_run_started_event_session_id_defaults_empty(): + """RunStartedEvent session_id defaults to empty string.""" + event = RunStartedEvent(run_id="r") + assert event.session_id == "" + + +# --------------------------------------------------------------------------- +# StreamCompleteEvent +# --------------------------------------------------------------------------- + + +def test_stream_complete_event_fields(): + """StreamCompleteEvent has the expected fields.""" + from agentpool.messaging.messages import ChatMessage + + msg = ChatMessage(content="final response", role="assistant") + event = StreamCompleteEvent(message=msg) + assert event.message is msg + assert event.cancelled is False + assert event.event_kind == "stream_complete" + + +def test_stream_complete_event_cancelled(): + """StreamCompleteEvent can be marked as cancelled.""" + from agentpool.messaging.messages import ChatMessage + + msg = ChatMessage(content="[Interrupted]", role="assistant") + event = StreamCompleteEvent(message=msg, cancelled=True) + assert event.cancelled is True + + +# --------------------------------------------------------------------------- +# ToolCallStartEvent +# --------------------------------------------------------------------------- + + +def test_tool_call_start_event_fields(): + """ToolCallStartEvent has the expected fields.""" + event = ToolCallStartEvent( + tool_call_id="tc-1", + tool_name="bash", + title="Executing: bash", + kind="execute", + raw_input={"command": "ls"}, + ) + assert event.tool_call_id == "tc-1" + assert event.tool_name == "bash" + assert event.title == "Executing: bash" + assert event.kind == "execute" + assert event.raw_input == {"command": "ls"} + assert event.event_kind == "tool_call_start" + assert event.session_id == "" + + +def test_tool_call_start_event_content_default_empty(): + """ToolCallStartEvent content defaults to empty list.""" + event = ToolCallStartEvent( + tool_call_id="tc-1", + tool_name="bash", + title="test", + kind="execute", + raw_input={}, + ) + assert event.content == [] + + +# --------------------------------------------------------------------------- +# ToolCallCompleteEvent +# --------------------------------------------------------------------------- + + +def test_tool_call_complete_event_fields(): + """ToolCallCompleteEvent has the expected fields.""" + event = ToolCallCompleteEvent( + tool_name="bash", + tool_call_id="tc-1", + tool_input={"command": "ls"}, + tool_result="file1\nfile2", + agent_name="my_agent", + message_id="msg-1", + ) + assert event.tool_name == "bash" + assert event.tool_call_id == "tc-1" + assert event.tool_input == {"command": "ls"} + assert event.tool_result == "file1\nfile2" + assert event.agent_name == "my_agent" + assert event.message_id == "msg-1" + assert event.event_kind == "tool_call_complete" + assert event.session_id == "" + + +def test_tool_call_complete_event_metadata_default_none(): + """ToolCallCompleteEvent metadata defaults to None.""" + event = ToolCallCompleteEvent( + tool_name="t", + tool_call_id="tc", + tool_input={}, + tool_result="r", + agent_name="a", + message_id="m", + ) + assert event.metadata is None + + +# --------------------------------------------------------------------------- +# RunFailedEvent +# --------------------------------------------------------------------------- + + +def test_run_failed_event_fields(): + """RunFailedEvent carries exception details.""" + exc = RuntimeError("something broke") + event = RunFailedEvent( + run_id="run-1", + session_id="sess-1", + exception=exc, + ) + assert event.run_id == "run-1" + assert event.session_id == "sess-1" + assert event.exception is exc + assert event.event_kind == "run_failed" + + +# --------------------------------------------------------------------------- +# RichAgentStreamEvent union +# --------------------------------------------------------------------------- + + +def test_rich_agent_stream_event_includes_custom_events(): + """RichAgentStreamEvent union includes all custom event types.""" + import typing + + # RichAgentStreamEvent may be a TypeAlias to a Union, so we need to + # resolve it via typing.get_origin / get_args or check __annotations__ + origin = typing.get_origin(RichAgentStreamEvent) + args = typing.get_args(RichAgentStreamEvent) + + # If it's a Union, args will be non-empty. If it's a TypeAlias to a + # class, we check differently. + if args: + assert RunStartedEvent in args or any( + RunStartedEvent is a for a in args + ) + else: + # May be a TypeAlias — check the underlying type + assert hasattr(RichAgentStreamEvent, "__value__") or hasattr( + RichAgentStreamEvent, "__origin__" + ) + + +# --------------------------------------------------------------------------- +# Event construction from RunExecutor patterns (without running executor) +# --------------------------------------------------------------------------- + + +def test_tool_call_start_event_from_tool_call_part(): + """ToolCallStartEvent can be constructed from a ToolCallPart-like input.""" + from pydantic_ai.messages import ToolCallPart + + call_part = ToolCallPart(tool_name="read", args={"path": "/tmp"}) + event = ToolCallStartEvent( + tool_call_id=call_part.tool_call_id, + tool_name=call_part.tool_name, + title=f"Executing: {call_part.tool_name}", + kind="read", + raw_input=dict(call_part.args) if isinstance(call_part.args, dict) else {}, + ) + assert event.tool_name == "read" + assert event.kind == "read" + + +if __name__ == "__main__": + pytest.main([__file__, "-v"]) diff --git a/tests/agents/test_process_history.py b/tests/agents/test_process_history.py new file mode 100644 index 000000000..6fd71bfc5 --- /dev/null +++ b/tests/agents/test_process_history.py @@ -0,0 +1,298 @@ +"""Regression tests for ``ProcessHistoryAdapter``. + +Tests cover the core wrapping logic, signature validation, annotation +detection, and processor ordering. These serve as a **behavioral baseline** +before the thinning refactor replaces the adapter with direct +``pydantic_ai.capabilities.ProcessHistory`` usage. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Any +import warnings + +import pytest +from pydantic_ai.capabilities import ProcessHistory +from pydantic_ai.messages import ModelMessage, ModelRequest, UserPromptPart +from pydantic_ai.tools import RunContext + +from agentpool.agents.native_agent.process_history_capability import ( + ProcessHistoryAdapter, + _is_run_context_annotation, +) + + +if TYPE_CHECKING: + from collections.abc import Callable, Sequence + + +# --------------------------------------------------------------------------- +# wrap_processor — single parameter (messages only) +# --------------------------------------------------------------------------- + + +def test_single_param_sync_passthrough(): + """Single-param sync processor is passed through unchanged.""" + + def processor(messages: list[ModelMessage]) -> list[ModelMessage]: + return messages + + wrapped = ProcessHistoryAdapter.wrap_processor(processor) + assert wrapped is processor # identity check + + +def test_single_param_async_passthrough(): + """Single-param async processor is passed through unchanged.""" + + async def processor(messages: list[ModelMessage]) -> list[ModelMessage]: + return messages + + wrapped = ProcessHistoryAdapter.wrap_processor(processor) + assert wrapped is processor + + +def test_single_param_untyped_passthrough(): + """Single-param untyped processor is passed through unchanged.""" + + def processor(messages): # noqa: ANN202 + return messages + + wrapped = ProcessHistoryAdapter.wrap_processor(processor) + assert wrapped is processor + + +# --------------------------------------------------------------------------- +# wrap_processor — two parameters with typed RunContext +# --------------------------------------------------------------------------- + + +def test_two_param_typed_runcontext_passthrough(): + """Two-param processor with explicit RunContext annotation passes through.""" + + def processor( + ctx: RunContext[Any], messages: list[ModelMessage] + ) -> list[ModelMessage]: + return messages + + wrapped = ProcessHistoryAdapter.wrap_processor(processor) + assert wrapped is processor + + +def test_two_param_generic_runcontext_passthrough(): + """Two-param processor with RunContext[Deps] passes through.""" + + def processor( + ctx: RunContext[str], messages: list[ModelMessage] + ) -> list[ModelMessage]: + return messages + + wrapped = ProcessHistoryAdapter.wrap_processor(processor) + assert wrapped is processor + + +def test_two_param_async_typed_runcontext_passthrough(): + """Two-param async processor with typed RunContext passes through.""" + + async def processor( + ctx: RunContext[Any], messages: list[ModelMessage] + ) -> list[ModelMessage]: + return messages + + wrapped = ProcessHistoryAdapter.wrap_processor(processor) + assert wrapped is processor + + +# --------------------------------------------------------------------------- +# wrap_processor — two parameters with UNTYPED first param +# --------------------------------------------------------------------------- + + +def test_two_param_untyped_sync_wrapped(): + """Two-param sync processor with untyped first param gets wrapped.""" + + def processor(ctx, messages): # noqa: ANN001, ANN202 + return messages + + wrapped = ProcessHistoryAdapter.wrap_processor(processor) + assert wrapped is not processor + assert hasattr(wrapped, "__wrapped__") + + +def test_two_param_untyped_async_wrapped(): + """Two-param async processor with untyped first param gets wrapped.""" + + async def processor(ctx, messages): # noqa: ANN001, ANN202 + return messages + + wrapped = ProcessHistoryAdapter.wrap_processor(processor) + assert wrapped is not processor + import inspect + + assert inspect.iscoroutinefunction(wrapped) + + +def test_wrapped_sync_preserves_behavior(): + """Wrapped sync processor still processes messages correctly.""" + + def processor(ctx, messages): # noqa: ANN001, ANN202 + # Simulate: append a marker message + return [*messages, ModelRequest(parts=[UserPromptPart(content="marker")])] + + wrapped = ProcessHistoryAdapter.wrap_processor(processor) + original_messages: list[ModelMessage] = [] + result = wrapped(None, original_messages) # ctx=None for test + assert len(result) == 1 + assert isinstance(result[0], ModelRequest) + + +async def test_wrapped_async_preserves_behavior(): + """Wrapped async processor still processes messages correctly.""" + + async def processor(ctx, messages): # noqa: ANN001, ANN202 + return [*messages, ModelRequest(parts=[UserPromptPart(content="async_marker")])] + + wrapped = ProcessHistoryAdapter.wrap_processor(processor) + result = await wrapped(None, []) + assert len(result) == 1 + + +# --------------------------------------------------------------------------- +# wrap_processor — invalid signatures +# --------------------------------------------------------------------------- + + +def test_zero_params_raises(): + """Processor with 0 params raises ValueError.""" + + def processor() -> list[ModelMessage]: # noqa: ANN201 + return [] + + with pytest.raises(ValueError, match="1 or 2 arguments"): + ProcessHistoryAdapter.wrap_processor(processor) + + +def test_three_params_raises(): + """Processor with 3 params raises ValueError.""" + + def processor(a, b, c): # noqa: ANN001, ANN202 + return [] + + with pytest.raises(ValueError, match="1 or 2 arguments"): + ProcessHistoryAdapter.wrap_processor(processor) + + +def test_four_params_raises(): + """Processor with 4 params raises ValueError.""" + + def processor(a, b, c, d): # noqa: ANN001, ANN202 + return [] + + with pytest.raises(ValueError, match="1 or 2 arguments"): + ProcessHistoryAdapter.wrap_processor(processor) + + +# --------------------------------------------------------------------------- +# _is_run_context_annotation +# --------------------------------------------------------------------------- + + +def test_is_run_context_bare(): + """Bare RunContext is detected.""" + assert _is_run_context_annotation(RunContext) is True + + +def test_is_run_context_generic(): + """RunContext[Deps] is detected.""" + assert _is_run_context_annotation(RunContext[Any]) is True + assert _is_run_context_annotation(RunContext[str]) is True + + +def test_is_run_context_not_runcontext(): + """Non-RunContext types are rejected.""" + assert _is_run_context_annotation(str) is False + assert _is_run_context_annotation(int) is False + assert _is_run_context_annotation(None) is False + + +def test_is_run_context_none_annotation(): + """None annotation is not RunContext.""" + assert _is_run_context_annotation(None) is False + + +# --------------------------------------------------------------------------- +# from_processors +# --------------------------------------------------------------------------- + + +def test_from_processors_empty(): + """Empty list returns empty list.""" + result = ProcessHistoryAdapter.from_processors([]) + assert result == [] + + +def test_from_processors_single(): + """Single processor produces single ProcessHistory.""" + def proc(messages: list[ModelMessage]) -> list[ModelMessage]: + return messages + + result = ProcessHistoryAdapter.from_processors([proc]) + assert len(result) == 1 + assert isinstance(result[0], ProcessHistory) + + +def test_from_processors_multiple_preserves_order(): + """Multiple processors produce ProcessHistory list in same order.""" + def proc1(messages: list[ModelMessage]) -> list[ModelMessage]: + return messages + def proc2(messages: list[ModelMessage]) -> list[ModelMessage]: + return messages + def proc3(messages: list[ModelMessage]) -> list[ModelMessage]: + return messages + + result = ProcessHistoryAdapter.from_processors([proc1, proc2, proc3]) + assert len(result) == 3 + assert all(isinstance(p, ProcessHistory) for p in result) + + +def test_from_processors_mixed_typed_untyped(): + """Mix of typed and untyped processors all get wrapped correctly.""" + def typed_proc( + ctx: RunContext[Any], messages: list[ModelMessage] + ) -> list[ModelMessage]: + return messages + + def untyped_proc(ctx, messages): # noqa: ANN001, ANN202 + return messages + + def single_proc(messages: list[ModelMessage]) -> list[ModelMessage]: + return messages + + result = ProcessHistoryAdapter.from_processors( + [typed_proc, untyped_proc, single_proc] + ) + assert len(result) == 3 + assert all(isinstance(p, ProcessHistory) for p in result) + + +# --------------------------------------------------------------------------- +# Integration: ProcessHistory capability works after wrapping +# --------------------------------------------------------------------------- + + +async def test_wrapped_processor_works_inside_process_history(): + """A wrapped untyped processor works correctly inside ProcessHistory.""" + call_log: list[str] = [] + + def processor(ctx, messages): # noqa: ANN001, ANN202 + call_log.append("called") + return messages + + wrapped = ProcessHistoryAdapter.wrap_processor(processor) + capability = ProcessHistory(wrapped) + + # ProcessHistory stores processors — verify it was accepted + assert isinstance(capability, ProcessHistory) + + +if __name__ == "__main__": + pytest.main([__file__, "-v"]) diff --git a/tests/agents/test_prompt_injection.py b/tests/agents/test_prompt_injection.py new file mode 100644 index 000000000..54f1f985d --- /dev/null +++ b/tests/agents/test_prompt_injection.py @@ -0,0 +1,305 @@ +"""Regression tests for ``PromptInjectionManager``. + +Tests cover the full lifecycle of both the inject/consume path +(tool result augmentation) and the queue/pop_queued/flush_pending_to_queue +path (follow-up prompts for ACP agents). + +These tests serve as a **behavioral baseline** before the thinning refactor +removes the queue/pop_queued/flush_pending_to_queue methods from the +native agent path. +""" + +from __future__ import annotations + +import pytest + +from agentpool.agents.prompt_injection import PromptInjectionManager + + +# --------------------------------------------------------------------------- +# inject / consume path +# --------------------------------------------------------------------------- + + +async def test_inject_then_consume_returns_xml_wrapped(): + """inject() stores message, consume() pops it wrapped in XML tags.""" + mgr = PromptInjectionManager() + mgr.inject("check tests") + + result = await mgr.consume() + assert result is not None + assert "" in result + assert "check tests" in result + assert "" in result + + +async def test_consume_returns_none_when_empty(): + """consume() returns None when no pending injections.""" + mgr = PromptInjectionManager() + assert await mgr.consume() is None + + +async def test_consume_is_fifo(): + """Multiple injections are consumed in FIFO order.""" + mgr = PromptInjectionManager() + mgr.inject("first") + mgr.inject("second") + mgr.inject("third") + + r1 = await mgr.consume() + r2 = await mgr.consume() + r3 = await mgr.consume() + + assert r1 is not None and "first" in r1 + assert r2 is not None and "second" in r2 + assert r3 is not None and "third" in r3 + + +async def test_consume_all_drains_all(): + """consume_all() drains all pending injections at once.""" + mgr = PromptInjectionManager() + mgr.inject("a") + mgr.inject("b") + + results = await mgr.consume_all() + assert len(results) == 2 + assert "a" in results[0] + assert "b" in results[1] + # all drained + assert not mgr.has_pending() + + +async def test_consume_all_empty_returns_empty_list(): + """consume_all() returns [] when no pending.""" + mgr = PromptInjectionManager() + assert await mgr.consume_all() == [] + + +def test_has_pending_true_after_inject(): + """has_pending() returns True after inject().""" + mgr = PromptInjectionManager() + assert not mgr.has_pending() + mgr.inject("x") + assert mgr.has_pending() + + +def test_has_pending_false_after_consume(): + """has_pending() returns False after all injections consumed.""" + mgr = PromptInjectionManager() + mgr.inject("x") + # consume is async but we can check state before + assert mgr.has_pending() + + +# --------------------------------------------------------------------------- +# queue / pop_queued path +# --------------------------------------------------------------------------- + + +def test_queue_then_pop_returns_prompts(): + """queue() stores prompts, pop_queued() returns them in FIFO order.""" + mgr = PromptInjectionManager() + mgr.queue("hello", "world") + + result = mgr.pop_queued() + assert result is not None + assert result == ("hello", "world") + assert not mgr.has_queued() + + +def test_pop_queued_returns_none_when_empty(): + """pop_queued() returns None when queue is empty.""" + mgr = PromptInjectionManager() + assert mgr.pop_queued() is None + + +def test_queue_multiple_groups_fifo(): + """Multiple queue() calls produce FIFO order on pop_queued().""" + mgr = PromptInjectionManager() + mgr.queue("first") + mgr.queue("second") + mgr.queue("third") + + assert mgr.pop_queued() == ("first",) + assert mgr.pop_queued() == ("second",) + assert mgr.pop_queued() == ("third",) + assert mgr.pop_queued() is None + + +def test_has_queued_true_after_queue(): + """has_queued() returns True after queue().""" + mgr = PromptInjectionManager() + assert not mgr.has_queued() + mgr.queue("x") + assert mgr.has_queued() + + +# --------------------------------------------------------------------------- +# insert_queued (front insertion) +# --------------------------------------------------------------------------- + + +def test_insert_queued_adds_at_front(): + """insert_queued() adds prompts at the front of the queue.""" + mgr = PromptInjectionManager() + mgr.queue("existing") + mgr.insert_queued(("priority",)) + + # priority should come out first + assert mgr.pop_queued() == ("priority",) + assert mgr.pop_queued() == ("existing",) + + +def test_insert_queued_on_empty_queue(): + """insert_queued() works on an empty queue.""" + mgr = PromptInjectionManager() + mgr.insert_queued(("first",)) + assert mgr.has_queued() + assert mgr.pop_queued() == ("first",) + + +# --------------------------------------------------------------------------- +# flush_pending_to_queue +# --------------------------------------------------------------------------- + + +def test_flush_moves_injections_to_queue(): + """flush_pending_to_queue() moves unconsumed injections to queued prompts.""" + mgr = PromptInjectionManager() + mgr.inject("unconsumed1") + mgr.inject("unconsumed2") + + assert mgr.has_pending() + assert not mgr.has_queued() + + mgr.flush_pending_to_queue() + + assert not mgr.has_pending() + assert mgr.has_queued() + + # Each injection becomes a single-element tuple + p1 = mgr.pop_queued() + p2 = mgr.pop_queued() + assert p1 == ("unconsumed1",) + assert p2 == ("unconsumed2",) + + +def test_flush_noop_when_no_pending(): + """flush_pending_to_queue() does nothing when no pending injections.""" + mgr = PromptInjectionManager() + mgr.flush_pending_to_queue() + assert not mgr.has_queued() + + +def test_flush_preserves_existing_queue(): + """flush_pending_to_queue() appends after existing queued prompts.""" + mgr = PromptInjectionManager() + mgr.queue("already_queued") + mgr.inject("pending") + + mgr.flush_pending_to_queue() + + # existing queue items come first + assert mgr.pop_queued() == ("already_queued",) + assert mgr.pop_queued() == ("pending",) + + +# --------------------------------------------------------------------------- +# clear +# --------------------------------------------------------------------------- + + +def test_clear_removes_everything(): + """clear() removes both pending injections and queued prompts.""" + mgr = PromptInjectionManager() + mgr.inject("p1") + mgr.queue("q1") + mgr.queue("q2") + + mgr.clear() + + assert not mgr.has_pending() + assert not mgr.has_queued() + assert mgr.pop_queued() is None + + +def test_clear_on_empty_manager(): + """clear() is safe on an empty manager.""" + mgr = PromptInjectionManager() + mgr.clear() # should not raise + assert not mgr.has_pending() + assert not mgr.has_queued() + + +def test_clear_idempotent(): + """clear() can be called multiple times safely.""" + mgr = PromptInjectionManager() + mgr.inject("x") + mgr.clear() + mgr.clear() # second call should be noop + assert not mgr.has_pending() + + +# --------------------------------------------------------------------------- +# Full lifecycle scenario +# --------------------------------------------------------------------------- + + +async def test_full_lifecycle_inject_consume_flush_pop(): + """Full lifecycle: inject → consume → flush → pop_queued.""" + mgr = PromptInjectionManager() + + # 1. Inject two messages + mgr.inject("msg1") + mgr.inject("msg2") + + # 2. Consume one (simulates tool hook consuming) + consumed = await mgr.consume() + assert consumed is not None + assert "msg1" in consumed + + # 3. Flush remaining unconsumed to queue + mgr.flush_pending_to_queue() + assert not mgr.has_pending() + assert mgr.has_queued() + + # 4. Pop from queue + popped = mgr.pop_queued() + assert popped == ("msg2",) + + # 5. Queue empty now + assert not mgr.has_queued() + + +async def test_xml_tag_format(): + """Verify the exact XML tag format produced by consume().""" + mgr = PromptInjectionManager() + mgr.inject("test content") + + result = await mgr.consume() + assert result == "\ntest content\n" + + +async def test_xml_tag_format_consume_all(): + """Verify the exact XML tag format produced by consume_all().""" + mgr = PromptInjectionManager() + mgr.inject("a") + mgr.inject("b") + + results = await mgr.consume_all() + assert results[0] == "\na\n" + assert results[1] == "\nb\n" + + +def test_repr_shows_counts(): + """__repr__ shows pending and queued counts.""" + mgr = PromptInjectionManager() + mgr.inject("x") + mgr.queue("y") + r = repr(mgr) + assert "pending=1" in r + assert "queued=1" in r + + +if __name__ == "__main__": + pytest.main([__file__, "-v"]) diff --git a/tests/hooks/test_hook_combining.py b/tests/hooks/test_hook_combining.py new file mode 100644 index 000000000..c9a2b5d06 --- /dev/null +++ b/tests/hooks/test_hook_combining.py @@ -0,0 +1,516 @@ +"""Regression tests for hook combination scenarios. + +These tests verify the deny > ask > allow priority semantics in +``AgentHooks._run_hooks`` — the core aggregation algorithm that +combines results from multiple parallel hooks. + +They serve as a **behavioral baseline** before the migration to +``pydantic_ai.capabilities.Hooks`` so we can prove parity. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Any +from unittest.mock import AsyncMock + +import pytest + +from agentpool.hooks import AgentHooks, CallableHook + + +if TYPE_CHECKING: + from agentpool.hooks import HookResult + + +# --------------------------------------------------------------------------- +# Helper hooks +# --------------------------------------------------------------------------- + +hook_calls: list[tuple[str, dict[str, Any]]] = [] + + +def _reset() -> None: + hook_calls.clear() + + +def _allow(**kw: Any) -> HookResult: + hook_calls.append(("allow", dict(kw))) + return {"decision": "allow"} + + +def _deny(**kw: Any) -> HookResult: + hook_calls.append(("deny", dict(kw))) + return {"decision": "deny", "reason": "no"} + + +def _ask(**kw: Any) -> HookResult: + hook_calls.append(("ask", dict(kw))) + return {"decision": "ask", "reason": "not sure"} + + +def _modify_input(**kw: Any) -> HookResult: + hook_calls.append(("modify", dict(kw))) + return {"decision": "allow", "modified_input": {"key": "val1"}} + + +def _modify_input_override(**kw: Any) -> HookResult: + hook_calls.append(("modify_override", dict(kw))) + return {"decision": "allow", "modified_input": {"key": "val2", "extra": True}} + + +def _additional_context(**kw: Any) -> HookResult: + hook_calls.append(("context", dict(kw))) + return {"decision": "allow", "additional_context": "ctx-A"} + + +def _additional_context_b(**kw: Any) -> HookResult: + hook_calls.append(("context_b", dict(kw))) + return {"decision": "allow", "additional_context": "ctx-B"} + + +def _modified_output(**kw: Any) -> HookResult: + hook_calls.append(("output", dict(kw))) + return {"decision": "allow", "modified_output": "REPLACED"} + + +def _continue_false(**kw: Any) -> HookResult: + hook_calls.append(("continue_false", dict(kw))) + return {"decision": "allow", "continue_": False} + + +def _raise_hook(**kw: Any) -> HookResult: + hook_calls.append(("raise", dict(kw))) + msg = "boom" + raise RuntimeError(msg) + + +# --------------------------------------------------------------------------- +# Priority: deny > ask > allow +# --------------------------------------------------------------------------- + + +async def test_priority_deny_over_allow(): + """A deny result from ANY hook overrides all allows.""" + _reset() + hooks = AgentHooks( + pre_run=[ + CallableHook(event="pre_run", fn=_allow), + CallableHook(event="pre_run", fn=_deny), + CallableHook(event="pre_run", fn=_allow), + ], + ) + result = await hooks.run_pre_run_hooks(agent_name="a", prompt="hi") + assert result["decision"] == "deny" + assert "no" in result.get("reason", "") + assert len(hook_calls) == 3 # all hooks executed + + +async def test_priority_deny_over_ask(): + """Deny overrides ask.""" + _reset() + hooks = AgentHooks( + pre_run=[ + CallableHook(event="pre_run", fn=_ask), + CallableHook(event="pre_run", fn=_deny), + ], + ) + result = await hooks.run_pre_run_hooks(agent_name="a", prompt="hi") + assert result["decision"] == "deny" + + +async def test_priority_ask_over_allow(): + """Ask takes effect only when no deny.""" + _reset() + hooks = AgentHooks( + pre_run=[ + CallableHook(event="pre_run", fn=_allow), + CallableHook(event="pre_run", fn=_ask), + ], + ) + result = await hooks.run_pre_run_hooks(agent_name="a", prompt="hi") + assert result["decision"] == "ask" + + +async def test_all_allow(): + """All allow → combined allow.""" + _reset() + hooks = AgentHooks( + pre_run=[ + CallableHook(event="pre_run", fn=_allow), + CallableHook(event="pre_run", fn=_allow), + ], + ) + result = await hooks.run_pre_run_hooks(agent_name="a", prompt="hi") + assert result["decision"] == "allow" + + +async def test_all_ask(): + """All ask → combined ask.""" + _reset() + hooks = AgentHooks( + pre_run=[ + CallableHook(event="pre_run", fn=_ask), + CallableHook(event="pre_run", fn=_ask), + ], + ) + result = await hooks.run_pre_run_hooks(agent_name="a", prompt="hi") + assert result["decision"] == "ask" + + +# --------------------------------------------------------------------------- +# Reason concatenation +# --------------------------------------------------------------------------- + + +async def test_reasons_concatenated_with_semicolon(): + """Multiple reasons are joined with '; '.""" + _reset() + hooks = AgentHooks( + pre_run=[ + CallableHook(event="pre_run", fn=_deny), + CallableHook(event="pre_run", fn=_ask), + ], + ) + result = await hooks.run_pre_run_hooks(agent_name="a", prompt="hi") + assert "no" in result["reason"] + assert "not sure" in result["reason"] + assert "; " in result["reason"] + + +# --------------------------------------------------------------------------- +# modified_input merging +# --------------------------------------------------------------------------- + + +async def test_modified_input_merges_across_hooks(): + """modified_input from multiple hooks merges via dict.update (later wins).""" + _reset() + hooks = AgentHooks( + pre_tool_use=[ + CallableHook(event="pre_tool_use", fn=_modify_input), + CallableHook(event="pre_tool_use", fn=_modify_input_override), + ], + ) + result = await hooks.run_pre_tool_hooks( + agent_name="a", tool_name="t", tool_input={"orig": 1} + ) + modified = result.get("modified_input") + assert modified is not None + assert modified["key"] == "val2" # second hook overrides + assert modified["extra"] is True # from second hook + assert modified.get("orig") is None # original tool_input not included + + +# --------------------------------------------------------------------------- +# modified_output — full replacement, later wins +# --------------------------------------------------------------------------- + + +async def test_modified_output_replacement(): + """modified_output is a full replacement; later hook wins.""" + _reset() + + def output_a(**kw: Any) -> HookResult: + return {"decision": "allow", "modified_output": "A"} + + def output_b(**kw: Any) -> HookResult: + return {"decision": "allow", "modified_output": "B"} + + hooks = AgentHooks( + post_tool_use=[ + CallableHook(event="post_tool_use", fn=output_a), + CallableHook(event="post_tool_use", fn=output_b), + ], + ) + result = await hooks.run_post_tool_hooks( + agent_name="a", + tool_name="t", + tool_input={}, + tool_output="original", + duration_ms=1.0, + ) + assert result.get("modified_output") == "B" + + +# --------------------------------------------------------------------------- +# additional_context concatenation +# --------------------------------------------------------------------------- + + +async def test_additional_context_concatenated_with_newline(): + """additional_context from multiple hooks joined with '\\n'.""" + _reset() + hooks = AgentHooks( + post_tool_use=[ + CallableHook(event="post_tool_use", fn=_additional_context), + CallableHook(event="post_tool_use", fn=_additional_context_b), + ], + ) + result = await hooks.run_post_tool_hooks( + agent_name="a", + tool_name="t", + tool_input={}, + tool_output="x", + duration_ms=0.0, + ) + ctx = result.get("additional_context", "") + assert "ctx-A" in ctx + assert "ctx-B" in ctx + assert "\n" in ctx + + +# --------------------------------------------------------------------------- +# continue_ aggregation +# --------------------------------------------------------------------------- + + +async def test_continue_false_from_any_hook(): + """If any hook sets continue_=False, combined result is False.""" + _reset() + hooks = AgentHooks( + post_tool_use=[ + CallableHook(event="post_tool_use", fn=_allow), + CallableHook(event="post_tool_use", fn=_continue_false), + ], + ) + result = await hooks.run_post_tool_hooks( + agent_name="a", + tool_name="t", + tool_input={}, + tool_output="x", + duration_ms=0.0, + ) + assert result.get("continue_") is False + + +async def test_continue_true_when_all_allow(): + """No hook sets continue_=False → not present in result.""" + _reset() + hooks = AgentHooks( + post_tool_use=[ + CallableHook(event="post_tool_use", fn=_allow), + ], + ) + result = await hooks.run_post_tool_hooks( + agent_name="a", + tool_name="t", + tool_input={}, + tool_output="x", + duration_ms=0.0, + ) + assert "continue_" not in result or result.get("continue_") is not False + + +# --------------------------------------------------------------------------- +# Exception resilience +# --------------------------------------------------------------------------- + + +async def test_exception_in_one_hook_does_not_block(): + """A hook that raises is logged and skipped, others still run.""" + _reset() + hooks = AgentHooks( + pre_run=[ + CallableHook(event="pre_run", fn=_allow), + CallableHook(event="pre_run", fn=_raise_hook), + CallableHook(event="pre_run", fn=_allow), + ], + ) + result = await hooks.run_pre_run_hooks(agent_name="a", prompt="hi") + assert result["decision"] == "allow" # raising hook skipped + assert len(hook_calls) == 3 # all three attempted + + +async def test_exception_with_deny_from_another(): + """Exception in one hook + deny from another → deny still wins.""" + _reset() + hooks = AgentHooks( + pre_run=[ + CallableHook(event="pre_run", fn=_raise_hook), + CallableHook(event="pre_run", fn=_deny), + ], + ) + result = await hooks.run_pre_run_hooks(agent_name="a", prompt="hi") + assert result["decision"] == "deny" + + +# --------------------------------------------------------------------------- +# Empty / no-match scenarios +# --------------------------------------------------------------------------- + + +async def test_empty_hooks_returns_allow(): + """No hooks configured → allow.""" + _reset() + hooks = AgentHooks() + result = await hooks.run_pre_run_hooks(agent_name="a", prompt="hi") + assert result["decision"] == "allow" + + +async def test_no_matching_hooks_returns_allow(): + """All hooks filtered out by matcher → allow.""" + _reset() + hooks = AgentHooks( + pre_tool_use=[ + CallableHook(event="pre_tool_use", fn=_deny, matcher="other_tool"), + ], + ) + result = await hooks.run_pre_tool_hooks( + agent_name="a", tool_name="my_tool", tool_input={} + ) + assert result["decision"] == "allow" + + +# --------------------------------------------------------------------------- +# CallableHook return type normalization +# --------------------------------------------------------------------------- + + +async def test_callable_hook_returns_none_defaults_to_allow(): + """CallableHook returning None → allow.""" + _reset() + + def returns_none(**kw: Any) -> None: + hook_calls.append(("none", dict(kw))) + + hooks = AgentHooks(pre_run=[CallableHook(event="pre_run", fn=returns_none)]) + result = await hooks.run_pre_run_hooks(agent_name="a", prompt="hi") + assert result["decision"] == "allow" + + +async def test_callable_hook_returns_string_becomes_additional_context(): + """CallableHook returning a string → allow + additional_context.""" + + def returns_str(**kw: Any) -> str: + return "injected text" + + hooks = AgentHooks(post_tool_use=[CallableHook(event="post_tool_use", fn=returns_str)]) + result = await hooks.run_post_tool_hooks( + agent_name="a", + tool_name="t", + tool_input={}, + tool_output="x", + duration_ms=0.0, + ) + assert result["decision"] == "allow" + assert result.get("additional_context") == "injected text" + + +async def test_callable_hook_returns_bool_true(): + """CallableHook returning True → allow.""" + + def returns_true(**kw: Any) -> bool: + return True + + hooks = AgentHooks(pre_run=[CallableHook(event="pre_run", fn=returns_true)]) + result = await hooks.run_pre_run_hooks(agent_name="a", prompt="hi") + assert result["decision"] == "allow" + + +async def test_callable_hook_returns_bool_false(): + """CallableHook returning False → deny.""" + + def returns_false(**kw: Any) -> bool: + return False + + hooks = AgentHooks(pre_run=[CallableHook(event="pre_run", fn=returns_false)]) + result = await hooks.run_pre_run_hooks(agent_name="a", prompt="hi") + assert result["decision"] == "deny" + + +# --------------------------------------------------------------------------- +# Matcher semantics +# --------------------------------------------------------------------------- + + +def test_matcher_star_matches_all(): + """matcher='*' compiles to None (matches all).""" + hook = CallableHook(event="pre_tool_use", fn=_allow, matcher="*") + assert hook._pattern is None + assert hook.matches({"event": "pre_tool_use", "tool_name": "anything"}) + + +def test_matcher_none_matches_all(): + """matcher=None compiles to None (matches all).""" + hook = CallableHook(event="pre_tool_use", fn=_allow, matcher=None) + assert hook._pattern is None + assert hook.matches({"event": "pre_tool_use", "tool_name": "anything"}) + + +def test_matcher_uses_search_not_fullmatch(): + """Matcher uses re.search (substring match).""" + hook = CallableHook(event="pre_tool_use", fn=_allow, matcher="bash") + assert hook.matches({"event": "pre_tool_use", "tool_name": "run_bash_command"}) + assert hook.matches({"event": "pre_tool_use", "tool_name": "bash"}) + assert not hook.matches({"event": "pre_tool_use", "tool_name": "python"}) + + +def test_matcher_ignored_for_non_tool_events(): + """Matcher only applies to pre_tool_use/post_tool_use events.""" + hook = CallableHook(event="pre_run", fn=_allow, matcher="bash") + # For pre_run, matcher is not checked — should match regardless + assert hook.matches({"event": "pre_run", "prompt": "hello"}) + + +def test_disabled_hook_does_not_match(): + """Disabled hook never matches.""" + hook = CallableHook(event="pre_run", fn=_allow, enabled=False) + assert not hook.matches({"event": "pre_run", "prompt": "hi"}) + + +# --------------------------------------------------------------------------- +# input_match semantics +# --------------------------------------------------------------------------- + + +def test_input_match_all_must_match(): + """All input_match patterns must match for the hook to trigger.""" + hook = CallableHook( + event="pre_tool_use", + fn=_allow, + matcher="task", + input_match={"mode": "^libarian$", "tag": "^plan$"}, + ) + assert hook.matches( + { + "event": "pre_tool_use", + "tool_name": "task", + "tool_input": {"mode": "libarian", "tag": "plan"}, + } + ) + assert not hook.matches( + { + "event": "pre_tool_use", + "tool_name": "task", + "tool_input": {"mode": "other", "tag": "plan"}, + } + ) + + +def test_input_match_missing_field_rejects(): + """Missing field in tool_input → no match.""" + hook = CallableHook( + event="pre_tool_use", + fn=_allow, + matcher="task", + input_match={"tag": "^plan$"}, + ) + assert not hook.matches( + {"event": "pre_tool_use", "tool_name": "task", "tool_input": {}} + ) + + +def test_input_match_no_tool_input_treated_as_empty(): + """Missing tool_input entirely → input_match sees empty dict → no match.""" + hook = CallableHook( + event="pre_tool_use", + fn=_allow, + matcher="task", + input_match={"tag": "^plan$"}, + ) + # tool_input key absent from HookInput + assert not hook.matches({"event": "pre_tool_use", "tool_name": "task"}) + + +if __name__ == "__main__": + pytest.main([__file__, "-v"]) diff --git a/tests/tools/test_tool_conversion.py b/tests/tools/test_tool_conversion.py new file mode 100644 index 000000000..82753cf23 --- /dev/null +++ b/tests/tools/test_tool_conversion.py @@ -0,0 +1,347 @@ +"""Regression tests for ``Tool.to_pydantic_ai()`` conversion. + +Tests cover the core conversion logic, schema handling, deferred metadata, +approval wrapping, and metadata assembly. These serve as a **behavioral +baseline** before the thinning refactor simplifies the conversion to a +direct 1:1 mapping. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Any + +import pytest +from pydantic_ai.tools import Tool as PydanticAiTool + +from agentpool.tools.base import FunctionTool, Tool, ToolResult, is_terminal_tool + + +if TYPE_CHECKING: + from collections.abc import Callable + + +# --------------------------------------------------------------------------- +# Helper to create a FunctionTool from a callable +# --------------------------------------------------------------------------- + + +def _make_tool(fn: Any, **kwargs: Any) -> FunctionTool[Any]: + """Create a FunctionTool from a callable with optional overrides.""" + return FunctionTool.from_callable(fn, **kwargs) + + +# --------------------------------------------------------------------------- +# Simple tool conversion +# --------------------------------------------------------------------------- + + +def test_simple_tool_converts_to_pydantic_ai(): + """A simple tool with no special features converts to a PydanticAiTool.""" + + def my_tool(x: int) -> str: + """A test tool.""" + return f"result: {x}" + + tool = _make_tool(my_tool, name_override="my_tool") + result = tool.to_pydantic_ai() + + assert isinstance(result, PydanticAiTool) + assert result.name == "my_tool" + + +def test_tool_name_preserved(): + """Tool name is preserved in conversion.""" + + def fn(x: str) -> str: + return x + + tool = _make_tool(fn, name_override="custom_name") + assert tool.to_pydantic_ai().name == "custom_name" + + +def test_tool_description_preserved(): + """Tool description is preserved in conversion.""" + + def fn(x: str) -> str: + """My description.""" + return x + + tool = _make_tool(fn, name_override="t") + converted = tool.to_pydantic_ai() + # The description may be stored differently, but the tool should have it + assert converted.name == "t" + + +# --------------------------------------------------------------------------- +# ToolResult fields +# --------------------------------------------------------------------------- + + +def test_tool_result_content_only(): + """ToolResult can be created with just content.""" + result = ToolResult(content="hello") + assert result.content == "hello" + assert result.structured_content is None + assert result.metadata is None + + +def test_tool_result_with_structured_content(): + """ToolResult structured_content field works.""" + result = ToolResult( + content="summary", + structured_content={"key": "value", "count": 42}, + ) + assert result.structured_content == {"key": "value", "count": 42} + + +def test_tool_result_with_metadata(): + """ToolResult metadata field works and is separate from content.""" + result = ToolResult( + content="visible to LLM", + metadata={"diff": "+added", "path": "/foo.py"}, + ) + assert result.metadata == {"diff": "+added", "path": "/foo.py"} + assert "diff" not in str(result.content) + + +def test_tool_result_all_fields(): + """ToolResult with all fields set.""" + result = ToolResult( + content="text", + structured_content={"data": 1}, + metadata={"ui_info": "extra"}, + ) + assert result.content == "text" + assert result.structured_content == {"data": 1} + assert result.metadata == {"ui_info": "extra"} + + +# --------------------------------------------------------------------------- +# ToolKind +# --------------------------------------------------------------------------- + + +def test_tool_kind_default_none(): + """Tool.category defaults to None.""" + def fn() -> str: + return "x" + + tool = _make_tool(fn) + assert tool.category is None + + +def test_tool_kind_can_be_set(): + """Tool.category can be set to any ToolKind value.""" + def fn() -> str: + return "x" + + for kind in ("read", "edit", "delete", "execute", "search", "other"): + tool = _make_tool(fn, category=kind) # type: ignore[arg-type] + assert tool.category == kind + + +def test_tool_kind_included_in_metadata(): + """Tool.category is included in metadata when converting to pydantic_ai.""" + def fn() -> str: + return "x" + + tool = _make_tool(fn, name_override="t", category="read") # type: ignore[arg-type] + converted = tool.to_pydantic_ai() + if hasattr(converted, "metadata") and converted.metadata: + assert converted.metadata.get("category") == "read" + + +# --------------------------------------------------------------------------- +# requires_confirmation / approval +# --------------------------------------------------------------------------- + + +def test_requires_confirmation_false_by_default(): + """Tool.requires_confirmation defaults to False.""" + def fn() -> str: + return "x" + + tool = _make_tool(fn) + assert tool.requires_confirmation is False + + +def test_requires_confirmation_propagates_to_requires_approval(): + """requires_confirmation=True → requires_approval=True on PydanticAiTool.""" + def fn() -> str: + return "x" + + tool = FunctionTool( + name="t", + description="d", + callable=fn, + requires_confirmation=True, + ) + converted = tool.to_pydantic_ai() + assert converted.requires_approval is True + + +def test_no_confirmation_means_no_approval(): + """requires_confirmation=False → requires_approval=False.""" + def fn() -> str: + return "x" + + tool = _make_tool(fn, name_override="t") + converted = tool.to_pydantic_ai() + assert converted.requires_approval is False + + +# --------------------------------------------------------------------------- +# Deferred tools +# --------------------------------------------------------------------------- + + +def test_deferred_unapproved_sets_requires_approval(): + """deferred=True + deferred_kind='unapproved' → requires_approval=True.""" + def fn() -> str: + return "x" + + tool = FunctionTool( + name="t", + description="d", + callable=fn, + deferred=True, + deferred_kind="unapproved", + ) + converted = tool.to_pydantic_ai() + assert converted.requires_approval is True + + +def test_deferred_external_does_not_set_requires_approval(): + """deferred=True + deferred_kind='external' → requires_approval=False.""" + def fn() -> str: + return "x" + + tool = FunctionTool( + name="t", + description="d", + callable=fn, + deferred=True, + deferred_kind="external", + ) + converted = tool.to_pydantic_ai() + assert converted.requires_approval is False + + +def test_deferred_strategy_stream_raises(): + """deferred_strategy='stream' raises NotImplementedError.""" + def fn() -> str: + return "x" + + with pytest.raises(NotImplementedError): + FunctionTool( + name="t", + description="d", + callable=fn, + deferred=True, + deferred_strategy="stream", + ) + + +def test_deferred_unapproved_with_continue_strategy_raises(): + """deferred_kind='unapproved' + deferred_strategy='continue' raises.""" + from agentpool.tools.exceptions import ToolError + + def fn() -> str: + return "x" + + with pytest.raises(ToolError, match="deferred_kind='unapproved' requires"): + FunctionTool( + name="t", + description="d", + callable=fn, + deferred=True, + deferred_kind="unapproved", + deferred_strategy="continue", + ) + + +# --------------------------------------------------------------------------- +# Terminal tool metadata +# --------------------------------------------------------------------------- + + +def test_terminal_tool_metadata_true(): + """Tool with agentpool_terminal=true in metadata is terminal.""" + def fn() -> str: + return "done" + + tool = FunctionTool( + name="t", + description="d", + callable=fn, + metadata={"agentpool_terminal": "true"}, + ) + assert is_terminal_tool(tool) is True + + +def test_terminal_tool_metadata_false(): + """Tool without terminal metadata is not terminal.""" + def fn() -> str: + return "done" + + tool = _make_tool(fn) + assert is_terminal_tool(tool) is False + + +def test_terminal_tool_metadata_various_true_values(): + """Various truthy values for agentpool_terminal.""" + from agentpool.tools.base import has_terminal_tool_metadata + + for val in ("1", "true", "yes", "on", "TRUE", "Yes"): + assert has_terminal_tool_metadata({"agentpool_terminal": val}) is True + + for val in ("0", "false", "no", "off", ""): + assert has_terminal_tool_metadata({"agentpool_terminal": val}) is False + + assert has_terminal_tool_metadata({}) is False + assert has_terminal_tool_metadata(None) is False + + +# --------------------------------------------------------------------------- +# Metadata assembly in to_pydantic_ai() +# --------------------------------------------------------------------------- + + +def test_agent_name_included_in_metadata(): + """agent_name is included in the metadata passed to PydanticAiTool.""" + def fn() -> str: + return "x" + + tool = FunctionTool( + name="t", + description="d", + callable=fn, + agent_name="my_agent", + ) + converted = tool.to_pydantic_ai() + if hasattr(converted, "metadata") and converted.metadata: + assert converted.metadata.get("agent_name") == "my_agent" + + +def test_custom_metadata_merged_with_agent_name_and_category(): + """Custom metadata is merged with agent_name and category.""" + def fn() -> str: + return "x" + + tool = FunctionTool( + name="t", + description="d", + callable=fn, + agent_name="agent1", + category="read", # type: ignore[arg-type] + metadata={"custom_key": "custom_val"}, + ) + converted = tool.to_pydantic_ai() + if hasattr(converted, "metadata") and converted.metadata: + assert converted.metadata.get("custom_key") == "custom_val" + assert converted.metadata.get("agent_name") == "agent1" + assert converted.metadata.get("category") == "read" + + +if __name__ == "__main__": + pytest.main([__file__, "-v"])