fix skills path - #13
Merged
Leoyzen merged 1 commit intoApr 9, 2026
Merged
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors skill path resolution by introducing ConfigPath to handle automatic path resolution during validation, deprecating the get_effective_paths method. Additionally, it improves observability by adding debug logging throughout the skill discovery and registration process. The reviewer noted that the base_path calculation in discover_skills is now redundant for path resolution and primarily serves logging purposes, which may be misleading.
Comment on lines
+123
to
+124
| base_path = config_file_path.parent if config_file_path is not None else UPath.cwd() | ||
| logger.debug("Starting skills discovery", base_path=str(base_path)) |
There was a problem hiding this comment.
The base_path is calculated here, but if config is provided, config.get_effective_paths() is called, which now explicitly ignores the config_file_path parameter for path resolution due to ConfigPath handling it automatically. This makes the base_path calculation on this line redundant for actual path resolution and only used for the debug log on the next line. It could be misleading. Consider removing this calculation and the associated debug log if base_path is not intended to influence path resolution in this function.
Leoyzen
added a commit
that referenced
this pull request
May 27, 2026
* 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）
Leoyzen
added a commit
that referenced
this pull request
Jun 3, 2026
- Wrap event_bus.unsubscribe() in try-except within finally block to prevent cleanup exceptions from masking original exceptions (Comment #10) - Remove dead code conditional publish in agent.py: process_tool_event() already publishes to EventBus internally when run_ctx.event_bus is set, so the if combined: block never executes (Comment #13) - Change EventBus subscription scope from "self" to "session" in subagent_tools.py for clarity and consistency (Comment #14)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
修复skills配置文件相对路径造成的路径问题