Feature/bump deps main 0402 - #7
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces significant refactoring and feature additions, including the introduction of the opencode_sdk for shared models, support for free-form text elicitation, and new experimental workspace/worktree management routes. My review identified a critical bug in the delete_message endpoint where modifying a list while iterating over it can cause unexpected behavior. I recommend using a list comprehension or iterating in reverse to safely remove items. Other comments regarding performance and event broadcasting were also provided for consideration.
| part_id=part.id, | ||
| ) | ||
| ) | ||
| messages.pop(i) |
There was a problem hiding this comment.
The delete_message function uses messages.pop(i) inside a loop while iterating over the same list. This is a common bug that can lead to skipping elements or IndexError. Use a list comprehension or iterate in reverse to safely remove items.
References
- Modifying a list while iterating over it can lead to unexpected behavior and bugs.
- _swap_session_agent: Update session agent registry after swap (review #5) - get_agent_role_config_option: Use display_name with type-safe fallback, add description (review #7) - list_modes: Use mode.id for programmatic identifiers, add explicit None guard (review #4, #8) - Update tests to match new behavior
* docs(rfc): add RFC-0034 ACP Session Config Options 统一化 新增 RFC-0034,提案升级 AgentPool ACP Server 的 Session Config Options 透出逻辑,使 Zed 等 ACP 兼容 IDE 能够选择模型和切换 Agent Role。 主要内容: - 识别 4 个 GAP:Agent Role 未透出(P0)、ACP/OpenCode model list 数据来源不一致(P1)、/mode 路由硬编码(P1)、get_session_mode_state 过滤过严(P2) - 分析 3 个方案,推荐选项 2(三阶段统一化) - 技术设计:build_model_state_for_acp()、get_agent_role_config_option()、 _swap_session_agent() 及 OpenCode /mode 路由动态修复 🤖 Generated with [Qoder][https://qoder.com] * docs(rfc): 根据 review 反馈修正 RFC-0034 - 修正 model fallback 逻辑:strict fallback(configured 存在时只用 configured) - 修正 agent_role current_value:使用 agent.name 而非 pool.main_agent.name - 重写 _swap_session_agent():委托 session.switch_active_agent() + _session_agent_locks 保护 - 增加 session._task_lock 协调:拒绝 active prompt 期间的 swap - 增加 pool.manifest null check 保护 - 修正 list_modes() null safety:state.agent 为 None 时返回默认值 - 明确开放问题 Q2/Q3 的决策:对话历史不继承、current_value 已修复 - 更新决策记录:增加 session mutation 复用、锁保护、task_lock 协调、对话历史决策 - 更新 Phase 2 实施计划:增加 Zed 预验证、并发测试、current_value 测试 - 调整工作量估算:~260 行 → ~240 行 * docs(rfc): RFC-0034 新增 Phase 0 — ACP Configurable LLM Providers 适配 ACP PR #648 (Configurable LLM Providers) 已 MERGED,引入 providers/list、providers/set、providers/disable 三个方法族, 允许客户端发现和覆盖 agent 的 LLM 请求路由。 主要更新: - 新增 GAP 5 (P0): providers/* 完全未实现 - 新增目标 G7: 实现 ACP providers/* 协议方法 - 新增 Phase 0: ProviderRouter 实现 + schema 类型定义 + ACP 请求处理器 + AgentCapabilities.providers 声明 - 修订 Phase 1: build_model_state_for_acp() 接受 provider_router 参数,过滤被禁用 provider 下的模型 - 更新架构概览图: 传输层(providers)与应用层(session config)分层 - 更新里程碑: 四阶段实施,Phase 0 优先于 Phase 1 - 新增开放问题 6/7/8: providers 对已运行 session 的影响、 provider 路由覆盖与 agent 初始化兼容、SessionModelState 中是否携带 provider 关联信息 - 新增决策记录: providers/set 保守策略、从 model_variants 派生 ProviderInfo、provider_router 参数解耦 🤖 Generated with [Qoder][https://qoder.com] * docs(rfc): 优化 RFC-0034 — 补充 Zed 源码级兼容性分析 基于 Zed 源码调研(crates/agent_ui/src/config_options.rs、profile_selector.rs、 agent_servers/src/acp.rs)的关键发现: 1. Zed 渲染所有 config_options 为独立 UI 按钮,agent_role 可正确显示和点击 2. first_config_option_id() 仅返回同 category 的第一个 option,键盘快捷键 可能冲突 — 标记为已知限制(NG7) 3. Zed ProfileSelector 完全独立于 ACP,使用本地 AgentSettings.profiles 4. Zed 当前完全不支持 providers/* 协议(Phase 0 暂无 Zed UI 入口) RFC 更新内容: - 新增 Zed IDE 渲染行为小节(源码级证据) - 新增 Zed 兼容性分析总结表 - 更新非目标 NG7:键盘快捷键冲突为已知限制 - 更新开放问题 5/6/7/8/9,标记 Zed 调研结论 - 更新 Phase 2 预验证:明确键盘限制和排序建议 - 更新向后兼容保证表:添加 category 冲突行 - 更新决策记录:补充 Zed 调研证据 🤖 Generated with [Qoder][https://qoder.com] * feat(acp): implement RFC-0034 ACP Session Config Options unification Phase 0: ACP Configurable LLM Providers - Add providers/* protocol methods (providers/list, providers/set, providers/disable) - Add ProviderRouter with override/disable/capability tracking - Add providers field to AgentCapabilities and InitializeResponse Phase 1: Shared Model List Logic - Add build_model_state_for_acp() with configured-first, tokonomics-fallback - Invert get_session_model_state() to use configured variants first Phase 2: Agent Role Config Option - Add get_agent_role_config_option() exposing pool.all_agents - Add _swap_session_agent() with lock protection - Extend set_session_config_option() with agent_role handling Phase 3: OpenCode /mode Route Fix - Dynamic /mode route using agent.get_modes() Also includes RFC-0033 MCP over ACP support: - Add AcpMcpServer type and acp field to McpCapabilities - Add acp_mcp_servers parameter to AgentCapabilities.create() Tests: - 35 new tests across provider_router, model_state, agent_role, config_routes, and cross-protocol integration - Snapshot tests re-baselined * chore: remove RFC-0033 code from RFC-0034 branch Remove accidentally included RFC-0033 MCP-over-ACP implementation: - Delete acp_mcp_manager.py, acp_mcp_transport.py - Delete RFC-0033 tests (test_mcp.py, test_acp_mcp_*, test_mcp_integration) - Remove AcpMcpServer from mcp.py - Remove acp field from McpCapabilities - Remove acp_mcp_servers parameter from AgentCapabilities.create() - Remove acp_mcp_servers parameter from InitializeResponse.create() - Remove RFC-0033 handler code from acp_agent.py Keep RFC-0034 changes intact: - providers/* protocol methods - ProviderRouter with override/disable - build_model_state_for_acp() configured-first logic - agent_role config option and swap - Dynamic /mode route * fix: address PR review comments for RFC-0034 - _swap_session_agent: Update session agent registry after swap (review #5) - get_agent_role_config_option: Use display_name with type-safe fallback, add description (review #7) - list_modes: Use mode.id for programmatic identifiers, add explicit None guard (review #4, #8) - Update tests to match new behavior * fix(agent): use model_variants in get_modes() instead of tokonomics Agent.get_modes() was calling get_available_models() which returns all tokonomics-discovered models (2000+). Now it checks configured model_variants first and only falls back to tokonomics when no variants are configured. Fixes the issue where config_options model selector showed thousands of models instead of the configured variants. * fix(agent): track model variant name to fix Zed Unknown display When using model_variants, get_modes() returned variant names as option ids but current_mode_id was the raw model identifier (e.g. openai:svc/glm-4.7). This caused Zed to display 'Unknown' because current_mode_id didn't match any available mode id. Fix: Add _current_model_variant field to Agent. When _set_mode() is called with a variant name, store it. get_modes() now uses _current_model_variant as current_mode_id so it matches the option ids. * fix(agent): set _current_model_variant on init when model is variant name Agent.__init__ resolves model string via _resolve_model_string(), but was not setting _current_model_variant. This caused get_modes() to fall back to self.model_name (the raw model identifier) on initial load, showing 'Unknown' in Zed until _set_mode() was called. Fix: Also track variant name in __init__ when model string matches a model_variants key. * fix(agent): use actual model identifier as mode id, variant name as display name Redesign model config option to use actual model identifiers: - id/value: actual model identifier (e.g. openai:svc/glm-4.7) - name: variant name (e.g. glm47) for display - current_mode_id: actual model identifier This ensures currentValue matches option values in Zed's config option selector, fixing the 'Unknown' display issue. _set_mode() now supports both actual model identifiers and variant names by reverse-lookup from manifest model_variants. * fix(agent): align get_modes() id format with model_name Use config.get_model().system:model_name for option ids instead of config.identifier, ensuring currentValue matches option values. Root cause: model_name returns pydantic-ai system:model_name format (e.g., 'openai:svc/glm-4.7') while config.identifier returns full provider format (e.g., 'openai-chat:svc/glm-4.7'), causing mismatch in Zed's model selector dropdown. * fix: address PR #37 review comments (round 2) - providers/set & providers/disable: 兼容 id 字段(Comment #12, #13) - provider_router: 防御性初始化 + 未知 provider 静默禁用(Comment #14, #15) - model_utils: 先过滤 raw toko_models(更准确),current_model 不在列表时插入(Comment #16) - .gitignore: 添加 .omo/(Comment #18)
#1 tool_call_id: QuestionCapability._question already uses replace() to propagate tool_name/tool_call_id/tool_input — verified with L2 test. #2 telemetry: add @logfire.instrument to QuestionCapability._question; background_task modules already instrumented. #3 state cleanup: after_run() evicts _session_states and _ephemeral_states, shuts down batcher and task manager. #5 queued cancel: pending cancel path fires on_completed before completion_event.set(). #6 flush exception: _flush catches broad Exception, marks delivered regardless of success/failure. #7 timeout message: CancelledError handler checks task.status == 'timed_out' before choosing message. #8 private API: guard pydantic_ai._agent_graph import with try/except and helpful error message. #9 L2 test: add test_question_tool_propagates_tool_call_id_to_agent_context. #10 error contract: _build_definition raises ValueError with descriptive message for missing name or non-dict input. Nit: remove DEBUG_TASK_MGR prefix, type coro as Coroutine, fix test names.
…Capability from xeno-agent (#346) * feat(capabilities): add QuestionCapability with YAML schema override support Move question tools into a proper AbstractCapability that accepts args.schemas and args.enabled_tools, mirroring BackgroundTaskCapability. Replaces the bare QuestionTools entry point so consumers can customize LLM-facing parameter descriptions via YAML schema files without writing their own capability wrapper. * test(question): migrate question tool tests from xeno-agent Move 51 question tool unit tests (40 for question_for_user + 11 for ask_followup_question) from xeno-agent to agentpool. Tests now import from agentpool_toolsets.builtin.question_tools instead of xeno_agent. Two assertions adjusted to match agentpool's actual implementation: - Error message regex: 'questionnaire' → 'questions' (agentpool naming) - ask_followup_question metadata: dropped suggestion_attributes check (agentpool's _format_followup_response doesn't emit this field) Also add RUF001 per-file ignore for CJK fullwidth punctuation in test data. * feat(question): merge simple question tool into QuestionCapability Add a 'question' tool to QuestionCapability that replicates the legacy QuestionTool behavior (simple prompt + optional response_schema via MCP Elicit). This unifies all user-interaction tools under one capability: - question_for_user: XML multi-question questionnaire - ask_followup_question: single question with <suggest> options - question: simplest single-question (replaces QuestionTool) Mark QuestionToolConfig (tools: [{type: question}]) as deprecated, directing users to capabilities: [{type: question}]. Add 3 new tests covering the question tool: default-enabled, enabled alone, enabled via schemas. * feat(capabilities): add BackgroundTaskCapability migrated from xeno-agent Migrate the complete BackgroundTaskCapability implementation to agentpool: - capability.py: full lifecycle management (task, background_output, background_cancel, steer_task tools) - manager.py: BackgroundTaskManager with concurrent task execution, cleanup, and session isolation - notification.py: NotificationBatcher for debounced completion notifications - types.py: BackgroundTask, SessionTaskState dataclasses - utils/tool_schema.py: YAML schema loading and LLM-facing schema override Includes 277 tests (unit + integration + resource provider) covering lifecycle, concurrency, error propagation, notification batching, history isolation, and cancellation regression. Config schema files (task.yaml, background_output.yaml, background_cancel.yaml, steer_task.yaml) provide default LLM-facing parameter descriptions. Entry point 'background_task' registered in pyproject.toml. * fix(ci): resolve ruff format, mypy, and flaky test failures Three CI failures fixed: 1. ruff format: question.py ternary expression reformatted 2. mypy (16 errors → 0): - tool_schema.py: cast yaml.safe_load/json.loads results to OpenAIFunctionDefinition, construct TypedDict with explicit key-value pairs instead of ** unpacking - question.py: add ToolResult return annotations to tool wrappers - manager.py: re-read task_model.status into locals after await to prevent mypy narrowing from concurrent status changes - capability.py: type session_pool as SessionPool | None, fix delivered bool assignment from followup() str|None return, remove dead config.type == 'team' comparison (agents dict never contains team configs), rename shadowed task_model variable 3. Flaky tests (8 failures): replace fixed asyncio.sleep(0.8) with _wait_until_called() polling helper in 3 test files. The fixed sleep was too tight under CI load — debounce timers fire late when the event loop is busy with parallel workers. * fix(ci): replace anyio TaskGroup with asyncio.ensure_future in NotificationBatcher Root cause: loop.call_later callbacks run in a separate contextvars context where anyio's sniffio async-library detection fails with AsyncLibraryNotFoundError. This prevented _schedule_flush from calling tg.start_soon, so _flush never executed and deliver_callback was never invoked — all 25 batcher tests + 8 notification tests failed in CI. Fix: replace anyio.create_task_group/start_soon with asyncio.ensure_future for scheduling flush coroutines. Keep anyio.CancelScope and anyio.fail_after for timeout protection (these don't require sniffio context). Track flush tasks in a set[asyncio.Task] and cancel/await them in shutdown. Also restore source_type team detection by checking config.type via str() cast (mypy-safe for AnyAgentConfig union that doesn't include team types at the type level, but mocks provide type='team' at runtime). * refactor(question): unify question tools into single question tool Merge ask_followup_question, question_for_user, and question into one unified question tool per reviewer feedback. The question_for_user implementation (richest, supports multi-question XML with enum/multi/ input types) is retained as the canonical implementation, renamed to question. ask_followup_question (legacy compat) and the simple question tool are removed. Changes: - question_tools.py: remove ask_followup_question + _format_followup_response, rename question_for_user to question - question.py: simplify QuestionCapability to expose only question - Update all tests, docs, and tool name references * fix(review): address opencode-agent review findings #1 tool_call_id: QuestionCapability._question already uses replace() to propagate tool_name/tool_call_id/tool_input — verified with L2 test. #2 telemetry: add @logfire.instrument to QuestionCapability._question; background_task modules already instrumented. #3 state cleanup: after_run() evicts _session_states and _ephemeral_states, shuts down batcher and task manager. #5 queued cancel: pending cancel path fires on_completed before completion_event.set(). #6 flush exception: _flush catches broad Exception, marks delivered regardless of success/failure. #7 timeout message: CancelledError handler checks task.status == 'timed_out' before choosing message. #8 private API: guard pydantic_ai._agent_graph import with try/except and helpful error message. #9 L2 test: add test_question_tool_propagates_tool_call_id_to_agent_context. #10 error contract: _build_definition raises ValueError with descriptive message for missing name or non-dict input. Nit: remove DEBUG_TASK_MGR prefix, type coro as Coroutine, fix test names. * chore: remove list_available_nodes tool (legacy, Leoyzen feedback #345) - src/agentpool_toolsets/builtin/subagent_tools.py: remove list_available_nodes method + create_tool registration - src/agentpool_config/toolsets.py: SubagentToolName Literal now only accepts 'task'; docstring updated - tests/toolsets/test_tool_filtering.py: update assertions - tests/toolsets/builtin/test_as_capability.py: update assertions - tests/servers/acp_server/test_claude_acp_toolset_integration.py: update assertion - tests/tools/test_runcontext.py: remove prompt referencing list_available_nodes (test was already xfail) - docs/how-to/advanced/acp-integration.md: update docs - docs/how-to/servers/mcp-server.md: update docs The tool was legacy code; Leoyzen noted agents list is now injected directly into system prompt. * fix: ruff format toolsets.py (single-entry Literal syntax)
20260402合并phi65最新main