From 424ba75a79ad2ce323c78a49a031d566eea253bc Mon Sep 17 00:00:00 2001 From: Moira Huang Date: Fri, 22 May 2026 13:03:31 -0700 Subject: [PATCH 1/4] Use local-or-remote locations for skills Co-Authored-By: Oz --- app/src/ai/agent/api/convert_conversation.rs | 8 +- app/src/ai/agent/api/convert_from.rs | 45 +++-- app/src/ai/agent/api/convert_from_tests.rs | 18 +- app/src/ai/agent/conversation.rs | 32 +++- app/src/ai/agent/task.rs | 79 ++++---- app/src/ai/agent/task_tests.rs | 10 +- app/src/ai/agent_sdk/driver.rs | 4 +- app/src/ai/agent_sdk/driver/output.rs | 8 +- .../action_model/execute/read_skill.rs | 2 +- .../action_model/execute/read_skill_tests.rs | 7 +- app/src/ai/blocklist/block.rs | 2 +- .../blocklist/block/view_impl/common_tests.rs | 3 +- .../ai/blocklist/block/view_impl/output.rs | 4 +- app/src/ai/blocklist/block_tests.rs | 5 +- app/src/ai/blocklist/controller.rs | 19 ++ .../ai/blocklist/controller/input_context.rs | 8 +- .../ai/blocklist/controller/shared_session.rs | 5 +- app/src/ai/blocklist/history_model.rs | 3 + .../blocklist/inline_action/code_diff_view.rs | 10 +- .../run_agents_card_view_tests.rs | 5 +- app/src/ai/skills/dummy_skill_manager.rs | 11 +- .../ai/skills/file_watchers/skill_watcher.rs | 47 +++-- .../file_watchers/skill_watcher_tests.rs | 56 +++--- app/src/ai/skills/file_watchers/utils.rs | 116 ++++++------ .../ai/skills/file_watchers/utils_tests.rs | 73 +++++--- app/src/ai/skills/global_skills.rs | 24 ++- app/src/ai/skills/global_skills_tests.rs | 8 +- app/src/ai/skills/resolve_skill_spec.rs | 9 +- app/src/ai/skills/skill_manager.rs | 93 +++++++--- app/src/ai/skills/skill_manager_tests.rs | 171 ++++++++++++++---- app/src/ai/skills/skill_utils.rs | 9 +- app/src/ai/skills/skill_utils_tests.rs | 59 ++++-- app/src/code/editor_management.rs | 31 ++-- .../ai_context_menu/skills/data_source.rs | 5 +- app/src/terminal/input/skills/data_source.rs | 5 +- app/src/terminal/input/slash_command_model.rs | 11 +- .../input/slash_commands/data_source/mod.rs | 10 +- .../slash_commands/data_source/zero_state.rs | 7 +- app/src/terminal/view.rs | 6 +- app/src/workspace/view.rs | 2 +- crates/ai/src/agent/action/convert.rs | 31 +--- crates/ai/src/skills/conversion.rs | 110 ++++++++++- crates/ai/src/skills/conversion_tests.rs | 100 ++++++++++ crates/ai/src/skills/mod.rs | 8 +- crates/ai/src/skills/parse_skill.rs | 116 +++++++----- crates/ai/src/skills/parse_skill_tests.rs | 3 +- crates/ai/src/skills/parser.rs | 15 -- crates/ai/src/skills/read_skills_tests.rs | 5 +- crates/ai/src/skills/skill_reference.rs | 10 +- crates/warp_util/src/local_or_remote_path.rs | 35 ++++ 50 files changed, 1015 insertions(+), 448 deletions(-) create mode 100644 crates/ai/src/skills/conversion_tests.rs diff --git a/app/src/ai/agent/api/convert_conversation.rs b/app/src/ai/agent/api/convert_conversation.rs index 19c05ddcc9..3d5436f3a1 100644 --- a/app/src/ai/agent/api/convert_conversation.rs +++ b/app/src/ai/agent/api/convert_conversation.rs @@ -12,7 +12,7 @@ use ai::agent::action_result::{ RequestComputerUseResult, SendMessageToAgentResult, StartAgentResult, StartAgentVersion, UseComputerResult, }; -use ai::skills::ParsedSkill; +use ai::skills::{ParsedSkill, SkillPathOrigin}; use chrono::{DateTime, Local, TimeZone}; use persistence::model::AgentConversationData; use warp_core::command::ExitCode; @@ -468,7 +468,10 @@ impl ConvertToExchanges for &api::Task { } api::message::Message::InvokeSkill(invoke_skill) => { if let Some(api_skill) = invoke_skill.skill.clone() { - if let Ok(parsed_skill) = ParsedSkill::try_from(api_skill) { + if let Ok(parsed_skill) = ParsedSkill::try_from_api_with_origin( + api_skill, + &SkillPathOrigin::RestoredDisplayOnly, + ) { let user_query = invoke_skill .user_query .clone() @@ -531,6 +534,7 @@ impl ConvertToExchanges for &api::Task { // TODO(alokedesai): Support persistence for the code review state. active_code_review: None, task_id: &TaskId::new(api_message.task_id.clone()), + skill_path_origin: &SkillPathOrigin::Unavailable, }) { current_outputs.push(output_msg); diff --git a/app/src/ai/agent/api/convert_from.rs b/app/src/ai/agent/api/convert_from.rs index 7fcc7263f6..708e2ec2b4 100644 --- a/app/src/ai/agent/api/convert_from.rs +++ b/app/src/ai/agent/api/convert_from.rs @@ -3,10 +3,13 @@ use std::collections::HashMap; use std::time::Duration; use ai::agent::action::LifecycleEventType as StartAgentLifecycleEventType; +use ai::agent::action::ReadSkillRequest; use ai::agent::action_result::StartAgentVersion; use ai::agent::convert::ToolToAIAgentActionError; use ai::agent::UnknownCitationTypeError; -use ai::skills::SkillReference; +use ai::skills::{ + skill_reference_from_api_skill_ref, skill_reference_from_read_skill_ref, SkillPathOrigin, +}; use api::ask_user_question::question::QuestionType; use warp_core::channel::ChannelState; use warp_multi_agent_api as api; @@ -50,6 +53,18 @@ impl TryFrom for AIAgentAttachment { } } +fn convert_read_skill( + read_skill: api::message::tool_call::ReadSkill, + skill_path_origin: &SkillPathOrigin, +) -> Result { + let Some(reference) = read_skill.skill_reference else { + return Err(ToolToAIAgentActionError::MissingSkillReference); + }; + let skill = skill_reference_from_read_skill_ref(reference, skill_path_origin) + .map_err(|_| ToolToAIAgentActionError::MissingSkillReference)?; + Ok(AIAgentActionType::ReadSkill(ReadSkillRequest { skill })) +} + /// Converts proto UserQueryMode to the internal UserQueryMode type pub(crate) fn convert_user_query_mode(mode: Option<&api::UserQueryMode>) -> UserQueryMode { let Some(mode) = mode else { @@ -120,7 +135,10 @@ fn convert_run_agents_execution_mode( } } -fn convert_run_agents(run_agents: api::RunAgents) -> AIAgentActionType { +fn convert_run_agents( + run_agents: api::RunAgents, + skill_path_origin: &SkillPathOrigin, +) -> AIAgentActionType { let api::RunAgents { summary, base_prompt, @@ -136,7 +154,7 @@ fn convert_run_agents(run_agents: api::RunAgents) -> AIAgentActionType { base_prompt, skills: skills .into_iter() - .filter_map(convert_skill_reference) + .filter_map(|skill| skill_reference_from_api_skill_ref(skill, skill_path_origin)) .collect(), model_id, harness_type: convert_run_agents_harness(harness.as_ref()).unwrap_or_default(), @@ -159,6 +177,7 @@ fn convert_run_agents(run_agents: api::RunAgents) -> AIAgentActionType { fn convert_start_agent_v2_execution_mode( execution_mode: Option, + skill_path_origin: &SkillPathOrigin, ) -> StartAgentExecutionMode { match execution_mode.and_then(|execution_mode| execution_mode.mode) { Some(api::start_agent_v2::execution_mode::Mode::Remote(remote)) => { @@ -167,7 +186,9 @@ fn convert_start_agent_v2_execution_mode( skill_references: remote .skills .into_iter() - .filter_map(convert_skill_reference) + .filter_map(|skill| { + skill_reference_from_api_skill_ref(skill, skill_path_origin) + }) .collect(), model_id: remote.model_id, computer_use_enabled: remote.computer_use_enabled, @@ -189,16 +210,6 @@ fn convert_start_agent_v2_execution_mode( } } -fn convert_skill_reference(skill_ref: api::SkillRef) -> Option { - match skill_ref.skill_reference { - Some(api::skill_ref::SkillReference::Path(path)) => Some(SkillReference::Path(path.into())), - Some(api::skill_ref::SkillReference::BundledSkillId(id)) => { - Some(SkillReference::BundledSkillId(id)) - } - None => None, - } -} - /// Unexpected errors when trying to convert an [`api::Message`] to an [`AIAgentOutputMessage`]. #[derive(Debug, thiserror::Error)] pub enum MessageToAIAgentOutputMessageError { @@ -233,6 +244,7 @@ pub struct ConversionParams<'a> { pub task_id: &'a TaskId, pub current_todo_list: Option<&'a AIAgentTodoList>, pub active_code_review: Option<&'a CodeReview>, + pub skill_path_origin: &'a SkillPathOrigin, } /// Trait for converting an [`api::Message`] to an [`AIAgentOutputMessage`]. @@ -828,6 +840,7 @@ impl ConvertAPIToolCallToAIAgentAction for api::message::ToolCall { prompt: start_agent.prompt, execution_mode: convert_start_agent_v2_execution_mode( start_agent.execution_mode, + params.skill_path_origin, ), lifecycle_subscription: start_agent.lifecycle_subscription.map( |subscription| { @@ -841,7 +854,7 @@ impl ConvertAPIToolCallToAIAgentAction for api::message::ToolCall { }) } api::message::tool_call::Tool::RunAgents(orchestrate) => { - create_standard_action(convert_run_agents(orchestrate)) + create_standard_action(convert_run_agents(orchestrate, params.skill_path_origin)) } api::message::tool_call::Tool::SendMessageToAgent(send_message) => { create_standard_action(AIAgentActionType::SendMessageToAgent { @@ -854,7 +867,7 @@ impl ConvertAPIToolCallToAIAgentAction for api::message::ToolCall { create_standard_action(insert_review_comments.into()) } api::message::tool_call::Tool::ReadSkill(read_skill) => { - create_standard_action(read_skill.try_into()?) + create_standard_action(convert_read_skill(read_skill, params.skill_path_origin)?) } api::message::tool_call::Tool::FetchConversation(fetch_conversation) => { create_standard_action(fetch_conversation.into()) diff --git a/app/src/ai/agent/api/convert_from_tests.rs b/app/src/ai/agent/api/convert_from_tests.rs index 7411474ec9..56cea34104 100644 --- a/app/src/ai/agent/api/convert_from_tests.rs +++ b/app/src/ai/agent/api/convert_from_tests.rs @@ -1,6 +1,8 @@ use ai::agent::action::AskUserQuestionType; -use ai::skills::SkillReference; +use ai::skills::{SkillPathOrigin, SkillReference}; +use std::path::PathBuf; use warp_multi_agent_api as api; +use warp_util::local_or_remote_path::LocalOrRemotePath; use super::{ convert_api_question, ConversionParams, ConvertAPIMessageToClientOutputMessage, @@ -299,6 +301,7 @@ fn converts_start_agent_tool_call_to_action_with_prompt() { task_id: &task_id, current_todo_list: None, active_code_review: None, + skill_path_origin: &SkillPathOrigin::Local, }) .expect("conversion should succeed"); @@ -328,6 +331,7 @@ fn converts_local_start_agent_v2_without_harness_type_to_defaults() { task_id: &task_id, current_todo_list: None, active_code_review: None, + skill_path_origin: &SkillPathOrigin::Local, }) .expect("conversion should succeed"); @@ -355,6 +359,7 @@ fn converts_upload_artifact_tool_call_to_action() { task_id: &task_id, current_todo_list: None, active_code_review: None, + skill_path_origin: &SkillPathOrigin::Local, }) .expect("conversion should succeed"); @@ -378,6 +383,7 @@ fn converts_file_artifact_created_message_with_filename() { task_id: &task_id, current_todo_list: None, active_code_review: None, + skill_path_origin: &SkillPathOrigin::Local, }) .expect("conversion should succeed"); @@ -408,6 +414,7 @@ fn converts_start_agent_tool_calls_with_different_prompt_lengths() { task_id: &task_id, current_todo_list: None, active_code_review: None, + skill_path_origin: &SkillPathOrigin::Local, }) .expect("partial conversion should succeed"); let updated_output = updated_message @@ -415,6 +422,7 @@ fn converts_start_agent_tool_calls_with_different_prompt_lengths() { task_id: &task_id, current_todo_list: None, active_code_review: None, + skill_path_origin: &SkillPathOrigin::Local, }) .expect("updated conversion should succeed"); @@ -443,6 +451,7 @@ fn converts_start_agent_with_explicit_empty_lifecycle_subscription() { task_id: &task_id, current_todo_list: None, active_code_review: None, + skill_path_origin: &SkillPathOrigin::Local, }) .expect("conversion should succeed"); @@ -472,6 +481,7 @@ fn converts_start_agent_with_cancelled_and_blocked_lifecycle_subscription() { task_id: &task_id, current_todo_list: None, active_code_review: None, + skill_path_origin: &SkillPathOrigin::Local, }) .expect("conversion should succeed"); @@ -504,6 +514,7 @@ fn converts_remote_start_agent_with_environment_id() { task_id: &task_id, current_todo_list: None, active_code_review: None, + skill_path_origin: &SkillPathOrigin::Local, }) .expect("conversion should succeed"); @@ -542,6 +553,7 @@ fn converts_remote_start_agent_v2_with_skill_references() { task_id: &task_id, current_todo_list: None, active_code_review: None, + skill_path_origin: &SkillPathOrigin::Local, }) .expect("conversion should succeed"); @@ -554,7 +566,7 @@ fn converts_remote_start_agent_v2_with_skill_references() { StartAgentExecutionMode::Remote { environment_id: "env-123".to_string(), skill_references: vec![ - SkillReference::Path("/tmp/SKILL.md".into()), + SkillReference::Path(LocalOrRemotePath::Local(PathBuf::from("/tmp/SKILL.md",))), SkillReference::BundledSkillId("review-comments".to_string()), ], model_id: "gpt-test".to_string(), @@ -583,6 +595,7 @@ fn converts_local_start_agent_v2_with_harness_type() { task_id: &task_id, current_todo_list: None, active_code_review: None, + skill_path_origin: &SkillPathOrigin::Local, }) .expect("conversion should succeed"); @@ -625,6 +638,7 @@ fn transfer_control_tool_call_converts_to_action_message() { task_id: &task_id, current_todo_list: None, active_code_review: None, + skill_path_origin: &SkillPathOrigin::Local, }) .expect("transfer-control conversion should succeed"); diff --git a/app/src/ai/agent/conversation.rs b/app/src/ai/agent/conversation.rs index d210d38643..b207bc22c8 100644 --- a/app/src/ai/agent/conversation.rs +++ b/app/src/ai/agent/conversation.rs @@ -3,6 +3,7 @@ use std::fmt::Display; use ai::agent::orchestration_config::{OrchestrationConfig, OrchestrationConfigStatus}; use ai::document::AIDocumentId; +use ai::skills::SkillPathOrigin; use chrono::{DateTime, Local, TimeZone}; use itertools::Itertools as _; use serde::{Deserialize, Serialize}; @@ -26,8 +27,8 @@ use super::api::ServerConversationToken; use super::task::helper::*; use super::task::transaction::{SavedTask, Transaction}; use super::task::{ - derive_todo_lists_from_root_task, ExtractMessagesError, Task, TaskId, UpdateTaskError, - UpgradeOptimisticTaskError, + derive_todo_lists_from_root_task, ExtractMessagesError, Task, TaskId, TaskMessageContext, + UpdateTaskError, UpgradeOptimisticTaskError, }; use super::task_store::TaskStore; use super::{ @@ -684,7 +685,7 @@ impl AIConversation { .remove(&root_task_id) .expect("root task should exist for upgrade-in-place test helper"); let server_root = root_task - .into_server_created_task(server_task, None, None, None) + .into_server_created_task(server_task, None, None, None, &SkillPathOrigin::Unavailable) .expect("upgrading optimistic root to a server-backed task should succeed"); self.task_store.set_root_task(server_root); } @@ -2282,6 +2283,7 @@ impl AIConversation { response_stream_id: &ResponseStreamId, terminal_view_id: EntityId, action: warp_multi_agent_api::client_action::Action, + skill_path_origin: &SkillPathOrigin, ctx: &mut ModelContext, ) -> Result<(), UpdateConversationError> { use warp_multi_agent_api::client_action::*; @@ -2330,6 +2332,7 @@ impl AIConversation { parent_task.source(), self.todo_lists.last(), self.code_review.as_ref(), + skill_path_origin, )?; ctx.emit(BlocklistAIHistoryEvent::UpgradedTask { optimistic_id: optimistic_id.clone(), @@ -2366,6 +2369,7 @@ impl AIConversation { existing_exchange, self.todo_lists.last(), self.code_review.as_ref(), + skill_path_origin, // In shared-session viewers, we have to reconstruct what the original user input // was using subsequent conversation messages (as the original input was not // sent on this client). Once we reconstruct these inputs, we will insert them @@ -2453,6 +2457,7 @@ impl AIConversation { None, self.todo_lists.last(), self.code_review.as_ref(), + skill_path_origin, )?; ctx.emit(BlocklistAIHistoryEvent::UpgradedTask { optimistic_id: old_id, @@ -2680,8 +2685,11 @@ impl AIConversation { task.add_messages( messages, exchange_id, - current_todo_list.as_ref(), - current_comment_state.as_ref(), + TaskMessageContext { + current_todo_list: current_todo_list.as_ref(), + active_code_review: current_comment_state.as_ref(), + skill_path_origin, + }, // In shared-session viewers, we have to reconstruct what the original user input // was using subsequent conversation messages (as the original input was not // sent on this client). Once we reconstruct these inputs, we will insert them @@ -2786,8 +2794,11 @@ impl AIConversation { task.upsert_message( message, exchange_id, - current_todo_list.as_ref(), - current_comment_state.as_ref(), + TaskMessageContext { + current_todo_list: current_todo_list.as_ref(), + active_code_review: current_comment_state.as_ref(), + skill_path_origin, + }, mask, is_viewing_shared_session, ) @@ -2831,8 +2842,11 @@ impl AIConversation { task.append_to_message_content( message, exchange_id, - current_todo_list.as_ref(), - current_comment_state.as_ref(), + TaskMessageContext { + current_todo_list: current_todo_list.as_ref(), + active_code_review: current_comment_state.as_ref(), + skill_path_origin, + }, mask, ) .map(|msg| msg.todos_op().cloned()) diff --git a/app/src/ai/agent/task.rs b/app/src/ai/agent/task.rs index fb4903e8f9..d56c2ceb94 100644 --- a/app/src/ai/agent/task.rs +++ b/app/src/ai/agent/task.rs @@ -5,6 +5,7 @@ use std::collections::{HashMap, HashSet}; use std::fmt::Display; use std::ops::Deref; +use ai::skills::SkillPathOrigin; use chrono::DateTime; use field_mask::{FieldMaskError, FieldMaskOperation}; use helper::{MessageExt, SubagentExt, ToolCallExt}; @@ -20,13 +21,13 @@ use super::api::convert_conversation::convert_tool_call_result_to_input; use super::api::{ user_inputs_from_messages, ConversionParams, ConvertAPIMessageToClientOutputMessage, }; +use super::comment::CodeReview; use super::conversation::{context_in_exchanges, update_todo_list_from_todo_op}; use super::{ AIAgentContext, AIAgentExchange, AIAgentExchangeId, AIAgentOutput, AIAgentOutputMessage, AIAgentOutputStatus, MaybeAIAgentOutputMessage, MessageId, MessageToAIAgentOutputMessageError, Shared, }; -use crate::ai::agent::comment::CodeReview; use crate::ai::document::ai_document_model::{AIDocumentId, AIDocumentVersion}; use crate::server::datetime_ext::DateTimeExt; use crate::terminal::model::block::BlockId; @@ -166,6 +167,12 @@ pub struct Task { /// List of `AIAgentExchange`s corresponding to messages contained in this task. exchanges: Vec, } +#[derive(Clone, Copy)] +pub(super) struct TaskMessageContext<'a> { + pub(super) current_todo_list: Option<&'a AIAgentTodoList>, + pub(super) active_code_review: Option<&'a CodeReview>, + pub(super) skill_path_origin: &'a SkillPathOrigin, +} impl Task { pub(super) fn new_optimistic_root() -> Self { @@ -193,6 +200,7 @@ impl Task { parent_task: Option<&api::Task>, current_todo_list: Option<&AIAgentTodoList>, active_code_review: Option<&CodeReview>, + skill_path_origin: &SkillPathOrigin, ) -> Result { match self.data { TaskImpl::Optimistic(optimistic::Task::Root) => { @@ -238,8 +246,11 @@ impl Task { if let Err(e) = self.update_exchange_from_messages( messages, exchange_id, - current_todo_list, - active_code_review, + TaskMessageContext { + current_todo_list, + active_code_review, + skill_path_origin, + }, false, ) { log::error!( @@ -272,7 +283,8 @@ impl Task { parent_task: &api::Task, existing_exchange: &AIAgentExchange, current_todo_list: Option<&AIAgentTodoList>, - current_comment_state: Option<&CodeReview>, + active_code_review: Option<&CodeReview>, + skill_path_origin: &SkillPathOrigin, should_convert_input_messages: bool, ) -> Self { let subagent_call_and_id = parent_task.messages.iter().find_map(|message| { @@ -325,8 +337,11 @@ impl Task { me.update_exchange_from_messages( messages_clone, new_exchange_id, - current_todo_list, - current_comment_state, + TaskMessageContext { + current_todo_list, + active_code_review, + skill_path_origin, + }, should_convert_input_messages, ) .expect("Exchange exists and output is in 'streaming' state."); @@ -647,8 +662,7 @@ impl Task { &mut self, messages: Vec, exchange_id: AIAgentExchangeId, - current_todo_list: Option<&AIAgentTodoList>, - current_comments: Option<&CodeReview>, + message_context: TaskMessageContext<'_>, should_convert_input_messages: bool, ) -> Result<(), UpdateTaskError> { if self.source().is_none() { @@ -657,8 +671,7 @@ impl Task { self.update_exchange_from_messages( messages.clone(), exchange_id, - current_todo_list, - current_comments, + message_context, should_convert_input_messages, )?; self.try_get_source_mut()?.messages.extend(messages); @@ -669,8 +682,7 @@ impl Task { &mut self, message: api::Message, exchange_id: AIAgentExchangeId, - current_todo_list: Option<&AIAgentTodoList>, - current_comments: Option<&CodeReview>, + message_context: TaskMessageContext<'_>, mask: FieldMask, should_convert_input_messages: bool, ) -> Result<&api::Message, UpdateTaskError> { @@ -684,8 +696,7 @@ impl Task { self.add_messages( vec![message.clone()], exchange_id, - current_todo_list, - current_comments, + message_context, should_convert_input_messages, )?; return self @@ -704,10 +715,13 @@ impl Task { .exchange_mut(exchange_id) .ok_or(UpdateTaskError::ExchangeNotFound)?; exchange_to_update.upsert_output_for_message( - &id, &updated_message, - current_todo_list, - current_comments, + ConversionParams { + task_id: &id, + current_todo_list: message_context.current_todo_list, + active_code_review: message_context.active_code_review, + skill_path_origin: message_context.skill_path_origin, + }, )?; // Task message updates can carry tool call result updates with them, @@ -752,8 +766,7 @@ impl Task { &mut self, message: api::Message, exchange_id: AIAgentExchangeId, - current_todo_list: Option<&AIAgentTodoList>, - current_comments: Option<&CodeReview>, + message_context: TaskMessageContext<'_>, mask: FieldMask, ) -> Result<&api::Message, UpdateTaskError> { let Some((idx, existing_message)) = self @@ -776,10 +789,13 @@ impl Task { .exchange_mut(exchange_id) .ok_or(UpdateTaskError::ExchangeNotFound)?; exchange_to_update.upsert_output_for_message( - &id, &updated_message, - current_todo_list, - current_comments, + ConversionParams { + task_id: &id, + current_todo_list: message_context.current_todo_list, + active_code_review: message_context.active_code_review, + skill_path_origin: message_context.skill_path_origin, + }, )?; let source = self.try_get_source_mut()?; @@ -904,8 +920,7 @@ impl Task { &mut self, messages: Vec, exchange_id: AIAgentExchangeId, - current_todo_list: Option<&AIAgentTodoList>, - active_code_review: Option<&CodeReview>, + message_context: TaskMessageContext<'_>, should_convert_input_messages: bool, ) -> Result<(), UpdateTaskError> { let exchange = self @@ -947,8 +962,9 @@ impl Task { .filter_map(|m| { match m.to_client_output_message(ConversionParams { task_id: &self.id, - current_todo_list, - active_code_review, + current_todo_list: message_context.current_todo_list, + active_code_review: message_context.active_code_review, + skill_path_origin: message_context.skill_path_origin, }) { Ok(MaybeAIAgentOutputMessage::Message(m)) => Some(Ok(m)), Ok(MaybeAIAgentOutputMessage::NoClientRepresentation) => None, @@ -983,10 +999,8 @@ impl AIAgentExchange { /// Note: this means updates will insert a new entry after previously added entries. fn upsert_output_for_message( &self, - task_id: &TaskId, task_message: &api::Message, - todo_list: Option<&AIAgentTodoList>, - comments: Option<&CodeReview>, + conversion_params: super::api::ConversionParams<'_>, ) -> Result<(), UpdateTaskError> { if let AIAgentOutputStatus::Streaming { output: Some(output), @@ -1000,11 +1014,8 @@ impl AIAgentExchange { match task_message .clone() - .to_client_output_message(ConversionParams { - current_todo_list: todo_list, - active_code_review: comments, - task_id, - })? { + .to_client_output_message(conversion_params)? + { MaybeAIAgentOutputMessage::Message(m) => { // Extract citations from the message and add to the output citations output.extend_citations(m.citations.clone()); diff --git a/app/src/ai/agent/task_tests.rs b/app/src/ai/agent/task_tests.rs index df5a2bd527..40f7c644be 100644 --- a/app/src/ai/agent/task_tests.rs +++ b/app/src/ai/agent/task_tests.rs @@ -1,10 +1,11 @@ use std::collections::HashSet; +use ai::skills::SkillPathOrigin; use chrono::Local; use prost_types::FieldMask; use warp_multi_agent_api as api; -use super::{ExtractMessagesError, Task}; +use super::{ExtractMessagesError, Task, TaskMessageContext}; use crate::ai::agent::{ AIAgentActionType, AIAgentExchange, AIAgentOutput, AIAgentOutputMessageType, AIAgentOutputStatus, MessageId, Shared, @@ -113,8 +114,11 @@ fn test_upsert_message_adds_start_agent_prompt_to_output() { "run tests", ), exchange_id, - None, - None, + TaskMessageContext { + current_todo_list: None, + active_code_review: None, + skill_path_origin: &SkillPathOrigin::Local, + }, FieldMask { paths: vec!["message.tool_call".to_string()], }, diff --git a/app/src/ai/agent_sdk/driver.rs b/app/src/ai/agent_sdk/driver.rs index 3a6c89a417..3cfd258bfe 100644 --- a/app/src/ai/agent_sdk/driver.rs +++ b/app/src/ai/agent_sdk/driver.rs @@ -1505,7 +1505,7 @@ impl AgentDriver { let load_skills_result = foreground .spawn(move |_, ctx| { - let skills = SkillWatcher::read_skills_for_repos(&repo_paths, ctx); + let skills = SkillWatcher::read_local_skills_for_repos(&repo_paths, ctx); if !skills.is_empty() { log::info!("Loaded {} environment skill(s)", skills.len()); } else { @@ -2148,7 +2148,7 @@ impl AgentDriver { .map(|parsed_skill| ResolvePromptAttachedSkill { name: parsed_skill.name.clone(), content: parsed_skill.content.clone(), - path: Some(parsed_skill.path.to_string_lossy().to_string()), + path: Some(parsed_skill.path.display_path()), }); let request = ResolvePromptRequest { skill, diff --git a/app/src/ai/agent_sdk/driver/output.rs b/app/src/ai/agent_sdk/driver/output.rs index e2d78e0d27..2f8454ddf2 100644 --- a/app/src/ai/agent_sdk/driver/output.rs +++ b/app/src/ai/agent_sdk/driver/output.rs @@ -1335,8 +1335,8 @@ fn format_agent_text(text: &AIAgentText, w: &mut W) -> io::Result<()> } match source { - Some(CodeSource::ProjectRules { path }) => { - writeln!(w, " rules_path={}", path.display())?; + Some(CodeSource::ProjectRules { location }) => { + writeln!(w, " rules_path={}", location.display_path())?; } Some(CodeSource::Link { path, @@ -1355,8 +1355,8 @@ fn format_agent_text(text: &AIAgentText, w: &mut W) -> io::Result<()> writeln!(w)?; } - Some(CodeSource::Skill { path, .. }) => { - writeln!(w, " skill_path={}", path.display())?; + Some(CodeSource::Skill { location, .. }) => { + writeln!(w, " skill_path={}", location.display_path())?; } Some(CodeSource::AIAction { .. }) | Some(CodeSource::New { .. }) diff --git a/app/src/ai/blocklist/action_model/execute/read_skill.rs b/app/src/ai/blocklist/action_model/execute/read_skill.rs index 367e85f17a..16ffc324db 100644 --- a/app/src/ai/blocklist/action_model/execute/read_skill.rs +++ b/app/src/ai/blocklist/action_model/execute/read_skill.rs @@ -47,7 +47,7 @@ impl ReadSkillExecutor { ctx ); let content = FileContext::new( - skill.path.to_string_lossy().into_owned(), + skill.path.display_path(), AnyFileContent::StringContent(skill.content.clone()), skill.line_range.clone(), None, diff --git a/app/src/ai/blocklist/action_model/execute/read_skill_tests.rs b/app/src/ai/blocklist/action_model/execute/read_skill_tests.rs index c21f0878dd..772c7ef173 100644 --- a/app/src/ai/blocklist/action_model/execute/read_skill_tests.rs +++ b/app/src/ai/blocklist/action_model/execute/read_skill_tests.rs @@ -8,6 +8,7 @@ use repo_metadata::watcher::DirectoryWatcher; use repo_metadata::RepoMetadataModel; use tempfile::TempDir; use warp_core::features::FeatureFlag; +use warp_util::local_or_remote_path::LocalOrRemotePath; use warpui::App; use watcher::HomeDirectoryWatcher; @@ -36,7 +37,7 @@ fn bundled_skill(name: &str) -> ParsedSkill { ParsedSkill { name: name.to_string(), description: format!("{name} bundled skill"), - path: PathBuf::from(format!("/bundled/skills/{name}/SKILL.md")), + path: LocalOrRemotePath::Local(PathBuf::from(format!("/bundled/skills/{name}/SKILL.md"))), content: format!("# {name}"), line_range: None, provider: SkillProvider::Warp, @@ -91,7 +92,7 @@ fn test_read_skill_executor_success() { let action = AIAgentAction { id: AIAgentActionId::from("test-action-id".to_string()), action: AIAgentActionType::ReadSkill(ReadSkillRequest { - skill: SkillReference::Path(skill_path.clone()), + skill: SkillReference::Path(LocalOrRemotePath::Local(skill_path.clone())), }), task_id: TaskId::new("test-task-id".to_string()), requires_result: false, @@ -173,7 +174,7 @@ fn test_read_skill_executor_file_not_found() { let action = AIAgentAction { id: AIAgentActionId::from("test-action-id".to_string()), action: AIAgentActionType::ReadSkill(ReadSkillRequest { - skill: SkillReference::Path(skill_path), + skill: SkillReference::Path(LocalOrRemotePath::Local(skill_path)), }), task_id: TaskId::new("test-task-id".to_string()), requires_result: false, diff --git a/app/src/ai/blocklist/block.rs b/app/src/ai/blocklist/block.rs index cf1e3558d5..2ee653fd52 100644 --- a/app/src/ai/blocklist/block.rs +++ b/app/src/ai/blocklist/block.rs @@ -3074,7 +3074,7 @@ impl AIBlock { ctx.emit(AIBlockEvent::OpenCodeInWarp { source: CodeSource::Skill { reference: reference.clone(), - path: path.clone(), + location: LocalOrRemotePath::Local(path.clone()), origin: SkillOpenOrigin::EditFiles, }, layout: *crate::util::file::external_editor::EditorSettings::as_ref( diff --git a/app/src/ai/blocklist/block/view_impl/common_tests.rs b/app/src/ai/blocklist/block/view_impl/common_tests.rs index 9b55a3d3de..b38b3840c0 100644 --- a/app/src/ai/blocklist/block/view_impl/common_tests.rs +++ b/app/src/ai/blocklist/block/view_impl/common_tests.rs @@ -5,6 +5,7 @@ use std::sync::Arc; use ai::skills::{ParsedSkill, SkillProvider, SkillScope}; use itertools::Itertools; use ui_components::lightbox::{LightboxImage, LightboxImageSource}; +use warp_util::local_or_remote_path::LocalOrRemotePath; #[cfg(feature = "local_fs")] use warpui::assets::asset_cache::AssetSource; use warpui::elements::{Empty, MouseStateHandle}; @@ -31,7 +32,7 @@ fn query_prefix_highlight_len_highlights_invoke_skill_inputs() { let input = AIAgentInput::InvokeSkill { context: Arc::new([]), skill: ParsedSkill { - path: PathBuf::from("/tmp/.agents/skills/review-pr/SKILL.md"), + path: LocalOrRemotePath::Local(PathBuf::from("/tmp/.agents/skills/review-pr/SKILL.md")), name: "review-pr".to_string(), description: "Review a pull request.".to_string(), content: String::new(), diff --git a/app/src/ai/blocklist/block/view_impl/output.rs b/app/src/ai/blocklist/block/view_impl/output.rs index 6de741bfd3..7ed38350fa 100644 --- a/app/src/ai/blocklist/block/view_impl/output.rs +++ b/app/src/ai/blocklist/block/view_impl/output.rs @@ -1739,7 +1739,7 @@ fn render_read_skill( if !skill.is_bundled() { let source = CodeSource::Skill { reference: skill_reference.clone(), - path: skill.path.clone(), + location: skill.path.clone(), origin: SkillOpenOrigin::ReadSkill, }; @@ -1844,7 +1844,7 @@ fn render_read_files( .reference_for_skill_path(&skill.path); let source = CodeSource::Skill { reference, - path: skill.path.clone(), + location: skill.path.clone(), origin: SkillOpenOrigin::ReadFiles, }; let skill_icon_override = icon_override_for_skill_name(&skill.name); diff --git a/app/src/ai/blocklist/block_tests.rs b/app/src/ai/blocklist/block_tests.rs index bb4f3765b2..182fde3151 100644 --- a/app/src/ai/blocklist/block_tests.rs +++ b/app/src/ai/blocklist/block_tests.rs @@ -16,6 +16,7 @@ use crate::ai::blocklist::action_model::{ }; use crate::settings::AISettings; use crate::test_util::settings::initialize_settings_for_tests; +use warp_util::local_or_remote_path::LocalOrRemotePath; #[test] fn reasoning_auto_collapses_when_user_has_not_manually_toggled() { @@ -207,7 +208,9 @@ fn agent_cfg() -> RunAgentsAgentRunConfig { fn remote_arm_propagates_skills_into_skill_references() { let skills = vec![ SkillReference::BundledSkillId("writing-pr-descriptions".to_string()), - SkillReference::Path(PathBuf::from("/tmp/skill/SKILL.md")), + SkillReference::Path(LocalOrRemotePath::Local(PathBuf::from( + "/tmp/skill/SKILL.md", + ))), ]; let mode = run_agents_to_start_agent_mode( &RunAgentsExecutionMode::Remote { diff --git a/app/src/ai/blocklist/controller.rs b/app/src/ai/blocklist/controller.rs index 168dc4e90c..d18b37e35b 100644 --- a/app/src/ai/blocklist/controller.rs +++ b/app/src/ai/blocklist/controller.rs @@ -14,6 +14,7 @@ use std::path::PathBuf; use std::sync::Arc; use std::time::Duration; +use ai::skills::SkillPathOrigin; use anyhow::anyhow; use chrono::{DateTime, Local}; use input_context::{input_context_for_request, parse_context_attachments}; @@ -123,6 +124,18 @@ impl SessionContext { matches!(self.session_type, Some(SessionType::WarpifiedRemote { .. })) } + pub fn skill_path_origin(&self) -> SkillPathOrigin { + match &self.session_type { + Some(SessionType::WarpifiedRemote { + host_id: Some(host_id), + }) => SkillPathOrigin::Remote { + host_id: host_id.clone(), + }, + Some(SessionType::WarpifiedRemote { host_id: None }) => SkillPathOrigin::Unavailable, + Some(SessionType::Local) | None => SkillPathOrigin::Local, + } + } + #[cfg(test)] pub fn new_for_test() -> Self { SessionContext { @@ -2613,6 +2626,11 @@ impl BlocklistAIController { } warp_multi_agent_api::response_event::Type::ClientActions(actions) => { let client_actions = actions.actions; + let skill_path_origin = SessionContext::from_session( + self.active_session.as_ref(ctx), + ctx, + ) + .skill_path_origin(); let apply_result = history_model.update(ctx, |history_model, ctx| { history_model.apply_client_actions( @@ -2620,6 +2638,7 @@ impl BlocklistAIController { client_actions, conversation_id, self.terminal_view_id, + &skill_path_origin, ctx, ) }); diff --git a/app/src/ai/blocklist/controller/input_context.rs b/app/src/ai/blocklist/controller/input_context.rs index 15459f9dd8..446cc82695 100644 --- a/app/src/ai/blocklist/controller/input_context.rs +++ b/app/src/ai/blocklist/controller/input_context.rs @@ -1,6 +1,4 @@ -use std::collections::HashMap; -use std::path::Path; -use std::sync::Arc; +use std::{collections::HashMap, sync::Arc}; use ai::index::full_source_code_embedding::manager::CodebaseIndexManager; use chrono::Local; @@ -76,7 +74,9 @@ pub(super) fn input_context_for_request( if FeatureFlag::ListSkills.is_enabled() { let skills = list_skills_if_changed( - active_session.current_working_directory().map(Path::new), + active_session + .current_working_directory_location(app) + .as_ref(), conversation_id, app, ); diff --git a/app/src/ai/blocklist/controller/shared_session.rs b/app/src/ai/blocklist/controller/shared_session.rs index f25f011723..473828fb13 100644 --- a/app/src/ai/blocklist/controller/shared_session.rs +++ b/app/src/ai/blocklist/controller/shared_session.rs @@ -12,7 +12,7 @@ use warp_multi_agent_api::response_event::{stream_finished, ClientActions}; use warpui::{AppContext, ModelContext, SingletonEntity}; use super::response_stream::ResponseStreamId; -use super::{BlocklistAIController, RequestInput}; +use super::{BlocklistAIController, RequestInput, SessionContext}; use crate::ai::agent::conversation::{AIConversationId, ConversationStatus}; use crate::ai::agent::{AIAgentActionId, AIAgentAttachment, EntrypointType}; use crate::ai::attachment_utils::{ @@ -312,6 +312,8 @@ impl BlocklistAIController { }; self.update_directory_context_from_client_actions(&actions, ctx); + let skill_path_origin = + SessionContext::from_session(self.active_session.as_ref(ctx), ctx).skill_path_origin(); let history_model = BlocklistAIHistoryModel::handle(ctx); history_model.update(ctx, |history_model, ctx| { if let Err(e) = history_model.apply_client_actions( @@ -319,6 +321,7 @@ impl BlocklistAIController { actions.actions, conversation_id, self.terminal_view_id, + &skill_path_origin, ctx, ) { log::error!( diff --git a/app/src/ai/blocklist/history_model.rs b/app/src/ai/blocklist/history_model.rs index b519e7ee91..ecbb577ed4 100644 --- a/app/src/ai/blocklist/history_model.rs +++ b/app/src/ai/blocklist/history_model.rs @@ -3,6 +3,7 @@ use std::collections::{HashMap, HashSet}; #[cfg(feature = "local_fs")] use std::sync::{Arc, Mutex}; +use ai::skills::SkillPathOrigin; use anyhow::anyhow; use chrono::{DateTime, Local, NaiveDateTime}; #[cfg(feature = "local_fs")] @@ -1524,6 +1525,7 @@ impl BlocklistAIHistoryModel { client_actions: Vec, conversation_id: AIConversationId, terminal_view_id: EntityId, + skill_path_origin: &SkillPathOrigin, ctx: &mut ModelContext, ) -> Result<(), UpdateHistoryError> { let mut current_conversation_id = conversation_id; @@ -1552,6 +1554,7 @@ impl BlocklistAIHistoryModel { response_stream_id, terminal_view_id, action, + skill_path_origin, ctx, )?; } diff --git a/app/src/ai/blocklist/inline_action/code_diff_view.rs b/app/src/ai/blocklist/inline_action/code_diff_view.rs index 1c82ab88ea..1511118505 100644 --- a/app/src/ai/blocklist/inline_action/code_diff_view.rs +++ b/app/src/ai/blocklist/inline_action/code_diff_view.rs @@ -1587,11 +1587,15 @@ impl CodeDiffView { let skill = common_path(&file_paths) .and_then(|common| skill_path_from_file_path(&common)) .and_then(|skill_path| SkillManager::as_ref(app).skill_by_path(&skill_path)); - if let Some(skill) = skill { - let skill_path = skill.path.clone(); + if let Some((skill, skill_path)) = skill.and_then(|skill| { + skill + .path + .to_local_path() + .map(|path| (skill, path.to_path_buf())) + }) { let skill_reference = SkillManager::handle(app) .as_ref(app) - .reference_for_skill_path(&skill_path); + .reference_for_skill_path(&skill.path); let skill_button_handle = self.button_mouse_states.skill_button_handle.clone(); let skill_icon_override = icon_override_for_skill_name(&skill.name); diff --git a/app/src/ai/blocklist/inline_action/run_agents_card_view_tests.rs b/app/src/ai/blocklist/inline_action/run_agents_card_view_tests.rs index 2bd0efbaff..5726deb0e3 100644 --- a/app/src/ai/blocklist/inline_action/run_agents_card_view_tests.rs +++ b/app/src/ai/blocklist/inline_action/run_agents_card_view_tests.rs @@ -6,6 +6,7 @@ use ai::agent::action_result::{ RunAgentsResult, }; use ai::skills::SkillReference; +use warp_util::local_or_remote_path::LocalOrRemotePath; use super::RunAgentsEditState; use crate::ai::blocklist::inline_action::orchestration_controls::OrchestrationEditState; @@ -223,7 +224,9 @@ fn to_request_round_trips_request_fields() { }, vec![ SkillReference::BundledSkillId("writing-pr-descriptions".to_string()), - SkillReference::Path(PathBuf::from("/tmp/skill/SKILL.md")), + SkillReference::Path(LocalOrRemotePath::Local(PathBuf::from( + "/tmp/skill/SKILL.md", + ))), ], ); req.plan_id = "plan-1".to_string(); diff --git a/app/src/ai/skills/dummy_skill_manager.rs b/app/src/ai/skills/dummy_skill_manager.rs index 050f437e96..cdff7ef428 100644 --- a/app/src/ai/skills/dummy_skill_manager.rs +++ b/app/src/ai/skills/dummy_skill_manager.rs @@ -1,6 +1,5 @@ -use std::path::Path; - use ai::skills::{ParsedSkill, SkillProvider, SkillReference}; +use warp_util::local_or_remote_path::LocalOrRemotePath; use warpui::{AppContext, Entity, ModelContext, SingletonEntity}; use crate::ai::skills::SkillDescriptor; @@ -14,18 +13,18 @@ impl SkillManager { pub fn get_skills_for_working_directory( &self, - _working_directory: Option<&Path>, + _working_directory: Option<&LocalOrRemotePath>, _ctx: &AppContext, ) -> Vec { vec![] } - pub fn skill_by_path(&self, _skill_path: &Path) -> Option<&ParsedSkill> { + pub fn skill_by_path(&self, _skill_path: &LocalOrRemotePath) -> Option<&ParsedSkill> { None } - pub fn reference_for_skill_path(&self, skill_path: &Path) -> SkillReference { - SkillReference::Path(skill_path.to_path_buf()) + pub fn reference_for_skill_path(&self, skill_path: &LocalOrRemotePath) -> SkillReference { + SkillReference::Path(skill_path.clone()) } pub fn skill_by_reference(&self, _reference: &SkillReference) -> Option<&ParsedSkill> { diff --git a/app/src/ai/skills/file_watchers/skill_watcher.rs b/app/src/ai/skills/file_watchers/skill_watcher.rs index 9e095e5380..363929774f 100644 --- a/app/src/ai/skills/file_watchers/skill_watcher.rs +++ b/app/src/ai/skills/file_watchers/skill_watcher.rs @@ -28,7 +28,7 @@ use crate::warp_managed_paths_watcher::{ #[derive(Debug, PartialEq)] pub enum SkillWatcherEvent { SkillsAdded { skills: Vec }, - SkillsDeleted { paths: Vec }, + SkillsDeleted { paths: Vec }, } pub struct SkillWatcher { // Channel for sending repository messages from subscribers. @@ -65,11 +65,14 @@ pub struct SkillWatcher { } impl SkillWatcher { - /// Synchronously reads skills from the given repo paths. + /// Synchronously reads skills from the given local repo paths. /// Requires file trees to already be built (i.e. `RepositoryUpdated` has fired). /// Returns the parsed skills; the caller is responsible for feeding them into /// `SkillManager::handle_skills_added`. - pub fn read_skills_for_repos(repo_paths: &[PathBuf], ctx: &AppContext) -> Vec { + pub fn read_local_skills_for_repos( + repo_paths: &[PathBuf], + ctx: &AppContext, + ) -> Vec { let repo_metadata = RepoMetadataModel::as_ref(ctx); let skill_files: Vec = repo_paths .iter() @@ -241,7 +244,10 @@ impl SkillWatcher { let _ = self .watcher_event_tx .try_send(SkillWatcherEvent::SkillsDeleted { - paths: deleted_paths, + paths: deleted_paths + .into_iter() + .map(LocalOrRemotePath::Local) + .collect(), }); } @@ -435,7 +441,10 @@ impl SkillWatcher { let _ = self .watcher_event_tx .try_send(SkillWatcherEvent::SkillsDeleted { - paths: deleted_paths, + paths: deleted_paths + .into_iter() + .map(LocalOrRemotePath::Local) + .collect(), }); } } @@ -515,7 +524,10 @@ impl SkillWatcher { let _ = self .watcher_event_tx .try_send(SkillWatcherEvent::SkillsDeleted { - paths: deleted_paths, + paths: deleted_paths + .into_iter() + .map(LocalOrRemotePath::Local) + .collect(), }); } } @@ -679,7 +691,10 @@ impl SkillWatcher { let _ = self .watcher_event_tx .try_send(SkillWatcherEvent::SkillsDeleted { - paths: deleted_paths, + paths: deleted_paths + .into_iter() + .map(LocalOrRemotePath::Local) + .collect(), }); } } @@ -714,18 +729,20 @@ impl SkillWatcher { /// via `DirectoryWatcher` so that modifications to the real file are detected. fn register_symlink_watches(&mut self, skills: &[ParsedSkill], ctx: &mut ModelContext) { for skill in skills { - let original_path = &skill.path; + let Some(original_path) = skill.path.to_local_path() else { + continue; + }; let Ok(canonical_path) = dunce::canonicalize(original_path) else { continue; }; - if canonical_path == *original_path { + if canonical_path == original_path { continue; // Not a symlink } self.symlink_canonical_to_originals .entry(canonical_path.clone()) .or_default() - .insert(original_path.clone()); + .insert(original_path.to_path_buf()); let Some(canonical_dir) = canonical_path.parent() else { continue; @@ -814,7 +831,10 @@ impl SkillWatcher { let _ = self .watcher_event_tx .try_send(SkillWatcherEvent::SkillsDeleted { - paths: deleted_original_paths, + paths: deleted_original_paths + .into_iter() + .map(LocalOrRemotePath::Local) + .collect(), }); } @@ -939,7 +959,10 @@ impl SkillWatcher { let _ = self .watcher_event_tx .try_send(SkillWatcherEvent::SkillsDeleted { - paths: deleted_paths, + paths: deleted_paths + .into_iter() + .map(LocalOrRemotePath::Local) + .collect(), }); } diff --git a/app/src/ai/skills/file_watchers/skill_watcher_tests.rs b/app/src/ai/skills/file_watchers/skill_watcher_tests.rs index 8cc999da9a..d310f85c4a 100644 --- a/app/src/ai/skills/file_watchers/skill_watcher_tests.rs +++ b/app/src/ai/skills/file_watchers/skill_watcher_tests.rs @@ -1,5 +1,6 @@ use std::collections::{HashMap, HashSet}; use std::fs; +use std::path::PathBuf; use super::super::subscribers::SkillRepositoryMessage; use super::SkillWatcher; @@ -12,7 +13,7 @@ use repo_metadata::{ DirectoryWatcher, RepoMetadataModel, RepositoryIdentifier, RepositoryUpdate, TargetFile, }; use tempfile::TempDir; -use warp_util::standardized_path::StandardizedPath; +use warp_util::{local_or_remote_path::LocalOrRemotePath, standardized_path::StandardizedPath}; use warpui::App; /// Helper function for creating a single skill file @@ -44,7 +45,7 @@ description: {} let line_range_start = skill_content.clone().lines().count() - content.lines().count() + 1; let line_range_end = skill_content.clone().lines().count() + 1; ParsedSkill { - path: skill_file_path, + path: LocalOrRemotePath::Local(skill_file_path), name: name.to_string(), description: description.to_string(), content: skill_content.clone(), @@ -54,6 +55,10 @@ description: {} } } +fn skill_local_path(skill: &ParsedSkill) -> PathBuf { + skill.path.to_local_path().unwrap().to_path_buf() +} + // ============================================================================ // Tests for handle_repository_update // ============================================================================ @@ -72,7 +77,7 @@ fn test_handle_repository_update_single_skill_added() { let skill = create_skill_file(&temp_dir, "test", "Test skill", "Test content"); let update = RepositoryUpdate { - added: HashSet::from([TargetFile::new(skill.path.clone(), false)]), + added: HashSet::from([TargetFile::new(skill_local_path(&skill), false)]), modified: HashSet::new(), deleted: HashSet::new(), moved: HashMap::new(), @@ -177,11 +182,14 @@ fn test_refresh_project_skills_for_repo_loads_symlinked_project_skill_directory( let symlink_parent = repo.join(".agents/skills"); fs::create_dir_all(&symlink_parent).unwrap(); let symlink_skill_dir = symlink_parent.join("linked-skill"); - std::os::unix::fs::symlink(target_skill.path.parent().unwrap(), &symlink_skill_dir) - .unwrap(); + std::os::unix::fs::symlink( + target_skill.path.to_local_path().unwrap().parent().unwrap(), + &symlink_skill_dir, + ) + .unwrap(); let mut expected_skill = target_skill; - expected_skill.path = symlink_skill_dir.join("SKILL.md"); + expected_skill.path = LocalOrRemotePath::Local(symlink_skill_dir.join("SKILL.md")); let repo_id = RepositoryIdentifier::try_local(&repo).unwrap(); let repo_key = StandardizedPath::try_from_local(&repo).unwrap(); @@ -262,9 +270,9 @@ fn test_local_project_fallback_scans_filesystem_when_repo_metadata_fails() { let SkillWatcherEvent::SkillsAdded { mut skills } = rx.recv().await.unwrap() else { panic!("Expected SkillsAdded event"); }; - skills.sort_by(|a, b| a.path.cmp(&b.path)); + skills.sort_by_key(|skill| skill.path.display_path()); let mut expected = vec![root_skill, subdir_skill]; - expected.sort_by(|a, b| a.path.cmp(&b.path)); + expected.sort_by_key(|skill| skill.path.display_path()); assert_eq!(skills, expected); }); } @@ -292,11 +300,14 @@ fn test_local_project_fallback_initial_scan_loads_symlinked_skill_directory() { let symlink_parent = repo.join(".agents/skills"); fs::create_dir_all(&symlink_parent).unwrap(); let symlink_skill_dir = symlink_parent.join("fallback-linked-skill"); - std::os::unix::fs::symlink(target_skill.path.parent().unwrap(), &symlink_skill_dir) - .unwrap(); + std::os::unix::fs::symlink( + skill_local_path(&target_skill).parent().unwrap(), + &symlink_skill_dir, + ) + .unwrap(); let mut expected_skill = target_skill; - expected_skill.path = symlink_skill_dir.join("SKILL.md"); + expected_skill.path = LocalOrRemotePath::Local(symlink_skill_dir.join("SKILL.md")); let repo_id = RepositoryIdentifier::try_local(&repo).unwrap(); skill_watcher_handle.update(&mut app, |skill_watcher, ctx| { @@ -325,7 +336,7 @@ fn test_local_project_fallback_update_reuses_repository_update_handler() { let skill = create_skill_file(&temp_dir, "fallback-update", "Fallback update", "Content"); let update = RepositoryUpdate { added: HashSet::new(), - modified: HashSet::from([TargetFile::new(skill.path.clone(), false)]), + modified: HashSet::from([TargetFile::new(skill_local_path(&skill), false)]), deleted: HashSet::new(), moved: HashMap::new(), commit_updated: false, @@ -403,7 +414,7 @@ fn test_handle_repository_update_skill_modified() { let update = RepositoryUpdate { added: HashSet::new(), - modified: HashSet::from([TargetFile::new(skill.path.clone(), false)]), + modified: HashSet::from([TargetFile::new(skill_local_path(&skill), false)]), deleted: HashSet::new(), moved: HashMap::new(), commit_updated: false, @@ -441,7 +452,7 @@ fn test_handle_repository_update_skill_deleted() { let update = RepositoryUpdate { added: HashSet::new(), modified: HashSet::new(), - deleted: HashSet::from([TargetFile::new(skill.path.clone(), false)]), + deleted: HashSet::from([TargetFile::new(skill_local_path(&skill), false)]), moved: HashMap::new(), commit_updated: false, index_lock_detected: false, @@ -480,8 +491,8 @@ fn test_handle_repository_update_multiple_skills_deleted() { added: HashSet::new(), modified: HashSet::new(), deleted: HashSet::from([ - TargetFile::new(skill_a.path.clone(), false), - TargetFile::new(skill_b.path.clone(), false), + TargetFile::new(skill_local_path(&skill_a), false), + TargetFile::new(skill_local_path(&skill_b), false), ]), moved: HashMap::new(), commit_updated: false, @@ -497,9 +508,9 @@ fn test_handle_repository_update_multiple_skills_deleted() { let SkillWatcherEvent::SkillsDeleted { mut paths } = event else { panic!("Expected SkillsDeleted event"); }; - paths.sort(); + paths.sort_by_key(LocalOrRemotePath::display_path); let mut expected = vec![skill_a.path, skill_b.path]; - expected.sort(); + expected.sort_by_key(LocalOrRemotePath::display_path); assert_eq!(paths, expected); }); } @@ -524,8 +535,8 @@ fn test_handle_repository_update_skill_moved() { modified: HashSet::new(), deleted: HashSet::new(), moved: HashMap::from([( - TargetFile::new(new_skill.path.clone(), false), - TargetFile::new(old_skill.path.clone(), false), + TargetFile::new(skill_local_path(&new_skill), false), + TargetFile::new(skill_local_path(&old_skill), false), )]), commit_updated: false, index_lock_detected: false, @@ -592,9 +603,10 @@ fn test_handle_repository_update_non_skill_directory_added_does_not_emit_project fn project_state(repo: &std::path::Path, skill: Option<&ParsedSkill>) -> FileTreeState { let children = if let Some(skill) = skill { - let skill_file = Entry::File(FileMetadata::new(skill.path.clone(), false)); + let skill_path = skill_local_path(skill); + let skill_file = Entry::File(FileMetadata::new(skill_path.clone(), false)); let skill_dir = Entry::Directory(DirectoryEntry { - path: StandardizedPath::try_from_local(skill.path.parent().unwrap()).unwrap(), + path: StandardizedPath::try_from_local(skill_path.parent().unwrap()).unwrap(), children: vec![skill_file], ignored: false, loaded: true, diff --git a/app/src/ai/skills/file_watchers/utils.rs b/app/src/ai/skills/file_watchers/utils.rs index 5ba2514aad..0107a9b572 100644 --- a/app/src/ai/skills/file_watchers/utils.rs +++ b/app/src/ai/skills/file_watchers/utils.rs @@ -1,19 +1,18 @@ -use std::collections::HashSet; use std::path::{Path, PathBuf}; -use std::sync::LazyLock; use ai::skills::{ home_skills_path, parse_skill, read_skills, ParsedSkill, SkillProvider, SKILL_PROVIDER_DEFINITIONS, }; use anyhow::Error; -use regex::Regex; -use repo_metadata::local_model::GetContentsArgs; -use repo_metadata::{RepoContent, RepoMetadataModel, RepositoryIdentifier}; +use repo_metadata::{ + local_model::GetContentsArgs, RepoContent, RepoMetadataModel, RepositoryIdentifier, +}; use walkdir::{DirEntry, WalkDir}; -use warp_util::local_or_remote_path::LocalOrRemotePath; -use warp_util::remote_path::RemotePath; -use warp_util::standardized_path::StandardizedPath; +use warp_util::{ + local_or_remote_path::LocalOrRemotePath, remote_path::RemotePath, + standardized_path::StandardizedPath, +}; use warpui::AppContext; use crate::warp_managed_paths_watcher::warp_managed_skill_dirs; @@ -30,39 +29,38 @@ fn local_or_remote_path_for_repo_path( } } -/// Finds all skill files in a repository by querying the RepoMetadataModel tree. +/// Finds concrete `SKILL.md` files in a repository tree. /// -/// Returns a list of paths to concrete `SKILL.md` files (e.g., -/// `/repo/.agents/skills/deploy/SKILL.md`, `/repo/sub/.claude/skills/build/SKILL.md`). +/// Remote sessions cannot walk the filesystem from the client, so callers use +/// repo metadata to locate files and then fetch contents through the daemon. pub fn find_skill_files_in_tree( repo_id: &RepositoryIdentifier, repo_metadata: &RepoMetadataModel, ctx: &AppContext, ) -> Vec { - // Filter during traversal: only collect concrete SKILL.md files that match a known provider - // path. This keeps project acquisition on repo metadata until local or remote file hydration. + let repo_id_for_filter = repo_id.clone(); let args = GetContentsArgs { include_folders: false, ..GetContentsArgs::default() } .include_ignored() - .with_filter(|content| { + .with_filter(move |content| { let RepoContent::File(file) = content else { return false; }; - is_skill_file(&file.path.to_local_path_lossy()) + let path = local_or_remote_path_for_repo_path(&repo_id_for_filter, &file.path); + extract_skill_parent_directory(&path).is_ok() }); + repo_metadata .get_repo_contents(repo_id, args, ctx) .unwrap_or_default() .into_iter() - // Only files should reach this iterator due to the GetContentsArgs::filter. - // Keep the Directory arm for exhaustive matching in case RepoContent grows new variants. - .filter_map(|content| match content { - RepoContent::File(file) => { - Some(local_or_remote_path_for_repo_path(repo_id, &file.path)) - } - RepoContent::Directory(_) => None, + .filter_map(|content| { + let RepoContent::File(file) = content else { + return None; + }; + Some(local_or_remote_path_for_repo_path(repo_id, &file.path)) }) .collect() } @@ -194,58 +192,58 @@ pub fn read_skills_from_files(skill_files: impl IntoIterator) -> } pub fn is_skill_file(path: &Path) -> bool { - extract_skill_parent_directory(path).is_ok() + extract_skill_parent_directory(&LocalOrRemotePath::Local(path.to_path_buf())).is_ok() } -static SKILL_PROVIDER_PATHS: LazyLock> = LazyLock::new(|| { - // Collect the skill provider paths from the definitions - SKILL_PROVIDER_DEFINITIONS - .iter() - .map(|p| p.skills_path.to_string_lossy().to_string()) - .collect() -}); - -// Pattern: {prefix}/{provider_path}/{skill-name}/SKILL.md -// where provider_path is 2 parts (e.g., ".agents/skills") and skill-name is 1 part -#[cfg(not(target_os = "windows"))] -static SKILL_FILE_PATTERN: LazyLock = LazyLock::new(|| { - Regex::new(r"(.+)/([^/]+/[^/]+)/[^/]+/SKILL\.md$") - .expect("Failed to compile skill file pattern") -}); - -// On windows, the path separator is \ -#[cfg(target_os = "windows")] -static SKILL_FILE_PATTERN: LazyLock = LazyLock::new(|| { - Regex::new(r"(.+)\\([^\\]+\\[^\\]+)\\[^\\]+\\SKILL\.md$") - .expect("Failed to compile skill file pattern") -}); - -pub fn extract_skill_parent_directory(path: &Path) -> Result { +pub fn extract_skill_parent_directory( + path: &LocalOrRemotePath, +) -> Result { let is_warp_home_skill = path - .file_name() + .to_local_path() + .and_then(Path::file_name) .and_then(|name| name.to_str()) .is_some_and(|name| name == "SKILL.md") && path - .parent() + .to_local_path() + .and_then(Path::parent) .and_then(Path::parent) .is_some_and(|parent| warp_managed_skill_dirs().iter().any(|dir| parent == dir)); if is_warp_home_skill { return dirs::home_dir() - .ok_or_else(|| anyhow::anyhow!("Home directory not available for {}", path.display())); + .map(LocalOrRemotePath::Local) + .ok_or_else(|| { + anyhow::anyhow!("Home directory not available for {}", path.display_path()) + }); + } + if path.file_name() != Some("SKILL.md") { + return Err(anyhow::anyhow!("Not a skill path: {}", path.display_path())); } - let path_str = path.to_string_lossy(); - if let Some(captures) = SKILL_FILE_PATTERN.captures(&path_str) { - if let Some(provider_path) = captures.get(2) { - if SKILL_PROVIDER_PATHS.contains(provider_path.as_str()) { - if let Some(parent_directory) = captures.get(1) { - return Ok(PathBuf::from(parent_directory.as_str())); - } - } + let Some(skill_dir) = path.parent() else { + return Err(anyhow::anyhow!("Not a skill path: {}", path.display_path())); + }; + let Some(skills_root) = skill_dir.parent() else { + return Err(anyhow::anyhow!("Not a skill path: {}", path.display_path())); + }; + + for definition in SKILL_PROVIDER_DEFINITIONS.iter() { + if !skills_root + .path_component() + .ends_with(&definition.skills_path.to_string_lossy()) + { + continue; + } + + let mut parent_directory = skills_root.clone(); + for _ in definition.skills_path.components() { + parent_directory = parent_directory + .parent() + .ok_or_else(|| anyhow::anyhow!("Not a skill path: {}", path.display_path()))?; } + return Ok(parent_directory); } - Err(anyhow::anyhow!("Not a skill path: {}", path.display())) + Err(anyhow::anyhow!("Not a skill path: {}", path.display_path())) } /// Check if this path is a skill directory under a home directory provider path diff --git a/app/src/ai/skills/file_watchers/utils_tests.rs b/app/src/ai/skills/file_watchers/utils_tests.rs index 69aa20d0a4..23411b77f8 100644 --- a/app/src/ai/skills/file_watchers/utils_tests.rs +++ b/app/src/ai/skills/file_watchers/utils_tests.rs @@ -1,18 +1,17 @@ -use repo_metadata::entry::{DirectoryEntry, Entry, FileMetadata}; -use repo_metadata::file_tree_store::FileTreeState; -use repo_metadata::file_tree_update::{ - DirectoryNodeMetadata, FileNodeMetadata, FileTreeEntryUpdate, RepoNodeMetadata, -}; -use repo_metadata::repositories::DetectedRepositories; use repo_metadata::{ + entry::{DirectoryEntry, Entry, FileMetadata}, + file_tree_store::FileTreeState, + file_tree_update::{ + DirectoryNodeMetadata, FileNodeMetadata, FileTreeEntryUpdate, RepoNodeMetadata, + }, + repositories::DetectedRepositories, DirectoryWatcher, RepoMetadataModel, RepoMetadataUpdate, RepositoryIdentifier, }; -use std::path::Path; use virtual_fs::{Stub, VirtualFS}; -use warp_util::host_id::HostId; -use warp_util::local_or_remote_path::LocalOrRemotePath; -use warp_util::remote_path::RemotePath; -use warp_util::standardized_path::StandardizedPath; +use warp_util::{ + host_id::HostId, local_or_remote_path::LocalOrRemotePath, remote_path::RemotePath, + standardized_path::StandardizedPath, +}; use warpui::App; use super::{ @@ -152,8 +151,11 @@ fn extract_skill_parent_directory_from_repo_root() { .join("skills") .join("my-skill") .join("SKILL.md"); - let result = extract_skill_parent_directory(&skill_path); - assert_eq!(result.ok(), Some(parent_directory)); + let result = extract_skill_parent_directory(&LocalOrRemotePath::Local(skill_path)); + assert_eq!( + result.ok(), + Some(LocalOrRemotePath::Local(parent_directory)) + ); } #[test] @@ -168,8 +170,11 @@ fn extract_skill_parent_directory_from_subdirectory() { .join("skills") .join("build") .join("SKILL.md"); - let result = extract_skill_parent_directory(&skill_path); - assert_eq!(result.ok(), Some(parent_directory)); + let result = extract_skill_parent_directory(&LocalOrRemotePath::Local(skill_path)); + assert_eq!( + result.ok(), + Some(LocalOrRemotePath::Local(parent_directory)) + ); } #[test] @@ -189,10 +194,10 @@ fn extract_skill_parent_directory_from_deep_subdirectory() { .join("skills") .join("test-skill") .join("SKILL.md"); - let result = extract_skill_parent_directory(&skill_path); + let result = extract_skill_parent_directory(&LocalOrRemotePath::Local(skill_path.clone())); assert_eq!( result.ok(), - Some(parent_directory), + Some(LocalOrRemotePath::Local(parent_directory)), "Failed for path: {}", skill_path.display() ); @@ -212,10 +217,10 @@ fn extract_skill_parent_directory_different_providers() { .join("skills") .join("s") .join("SKILL.md"); - let result = extract_skill_parent_directory(&path); + let result = extract_skill_parent_directory(&LocalOrRemotePath::Local(path.clone())); assert_eq!( result.ok(), - Some(repo.clone()), + Some(LocalOrRemotePath::Local(repo.clone())), "Failed for path: {}", path.display() ); @@ -236,7 +241,10 @@ fn extract_skill_parent_directory_returns_none_for_non_skill() { .join("skills") .join("my-skill") .join("README.md"); - assert_eq!(extract_skill_parent_directory(&path).ok(), None); + assert_eq!( + extract_skill_parent_directory(&LocalOrRemotePath::Local(path)).ok(), + None + ); // Wrong structure (skill directly in skills dir) let path = home_dir @@ -244,7 +252,10 @@ fn extract_skill_parent_directory_returns_none_for_non_skill() { .join(".agents") .join("skills") .join("SKILL.md"); - assert_eq!(extract_skill_parent_directory(&path).ok(), None); + assert_eq!( + extract_skill_parent_directory(&LocalOrRemotePath::Local(path)).ok(), + None + ); // Too deeply nested let path = home_dir @@ -254,11 +265,17 @@ fn extract_skill_parent_directory_returns_none_for_non_skill() { .join("a") .join("b") .join("SKILL.md"); - assert_eq!(extract_skill_parent_directory(&path).ok(), None); + assert_eq!( + extract_skill_parent_directory(&LocalOrRemotePath::Local(path)).ok(), + None + ); // Not in a skills directory let path = home_dir.join("repo").join("src").join("SKILL.md"); - assert_eq!(extract_skill_parent_directory(&path).ok(), None); + assert_eq!( + extract_skill_parent_directory(&LocalOrRemotePath::Local(path)).ok(), + None + ); } // ============================================================================ @@ -364,8 +381,8 @@ fn extract_skill_parent_directory_returns_home_dir_for_warp_home_skill() { }; let skill_path = warp_home_skills_dir.join("test-skill").join("SKILL.md"); - let result = extract_skill_parent_directory(&skill_path); - assert_eq!(result.ok(), Some(home_dir)); + let result = extract_skill_parent_directory(&LocalOrRemotePath::Local(skill_path)); + assert_eq!(result.ok(), Some(LocalOrRemotePath::Local(home_dir))); } #[test] @@ -537,7 +554,8 @@ fn find_skill_files_in_tree_finds_root_skills() { let local_skill_files = skill_files .into_iter() - .filter_map(|path| path.to_local_path().map(Path::to_path_buf)); + .filter_map(|path| path.to_local_path().map(|path| path.to_path_buf())) + .collect::>(); let skills = read_skills_from_files(local_skill_files); assert_eq!(skills.len(), 2); let names: Vec<&str> = skills.iter().map(|s| s.name.as_str()).collect(); @@ -689,7 +707,8 @@ fn find_skill_files_in_tree_finds_subdirectory_skills() { let local_skill_files = skill_files .into_iter() - .filter_map(|path| path.to_local_path().map(Path::to_path_buf)); + .filter_map(|path| path.to_local_path().map(|path| path.to_path_buf())) + .collect::>(); let skills = read_skills_from_files(local_skill_files); assert_eq!(skills.len(), 2); let names: Vec<&str> = skills.iter().map(|s| s.name.as_str()).collect(); diff --git a/app/src/ai/skills/global_skills.rs b/app/src/ai/skills/global_skills.rs index 252374856d..3e580cffa4 100644 --- a/app/src/ai/skills/global_skills.rs +++ b/app/src/ai/skills/global_skills.rs @@ -1,12 +1,15 @@ //! Helpers for resolving per-agent "global" skill specs into repos to //! ensure are available on disk before the agent runs. -use std::collections::{BTreeSet, HashMap, HashSet}; -use std::path::{Path, PathBuf}; -use std::str::FromStr; +use std::{ + collections::{BTreeSet, HashMap, HashSet}, + path::Path, + str::FromStr, +}; use ai::skills::{provider_rank, ParsedSkill}; use warp_cli::skill::SkillSpec; +use warp_util::local_or_remote_path::LocalOrRemotePath; use crate::ai::cloud_environments::GithubRepo; @@ -78,11 +81,11 @@ pub fn filter_skills_by_spec( fn matching_skill_path( repo_path: &Path, - skills_by_path: &HashMap, + skills_by_path: &HashMap, spec: &SkillSpec, -) -> Option { +) -> Option { if spec.is_full_path() { - let path = repo_path.join(&spec.skill_identifier); + let path = LocalOrRemotePath::Local(repo_path.join(&spec.skill_identifier)); return skills_by_path.contains_key(&path).then_some(path); } matching_simple_skill_path(repo_path, skills_by_path, &spec.skill_identifier) @@ -90,19 +93,20 @@ fn matching_skill_path( fn matching_simple_skill_path( repo_path: &Path, - skills_by_path: &HashMap, + skills_by_path: &HashMap, skill_name: &str, -) -> Option { +) -> Option { + let repo_path = LocalOrRemotePath::Local(repo_path.to_path_buf()); let mut matches = skills_by_path .values() .copied() - .filter(|skill| skill.path.starts_with(repo_path) && skill.name == skill_name) + .filter(|skill| skill.path.starts_with(&repo_path) && skill.name == skill_name) .collect::>(); matches.sort_by(|left, right| { provider_rank(left.provider) .cmp(&provider_rank(right.provider)) - .then_with(|| left.path.cmp(&right.path)) + .then_with(|| left.path.display_path().cmp(&right.path.display_path())) }); matches.into_iter().map(|skill| skill.path.clone()).next() } diff --git a/app/src/ai/skills/global_skills_tests.rs b/app/src/ai/skills/global_skills_tests.rs index fc1c5bce4d..636fd393da 100644 --- a/app/src/ai/skills/global_skills_tests.rs +++ b/app/src/ai/skills/global_skills_tests.rs @@ -3,6 +3,7 @@ use std::str::FromStr; use ai::skills::{get_provider_for_path, ParsedSkill, SkillProvider, SkillScope}; use warp_cli::skill::SkillSpec; +use warp_util::local_or_remote_path::LocalOrRemotePath; use super::{filter_skills_by_spec, resolve_skill_repos}; use crate::ai::cloud_environments::GithubRepo; @@ -149,7 +150,7 @@ fn skill_path(repo_path: &Path, provider_dir: &str, skill_name: &str) -> PathBuf fn parsed_skill(path: PathBuf, name: &str) -> ParsedSkill { let provider = get_provider_for_path(&path).unwrap_or(SkillProvider::Agents); ParsedSkill { - path, + path: LocalOrRemotePath::Local(path), name: name.to_string(), description: String::new(), content: String::new(), @@ -160,7 +161,10 @@ fn parsed_skill(path: PathBuf, name: &str) -> ParsedSkill { } fn skill_paths(skills: Vec) -> Vec { - skills.into_iter().map(|skill| skill.path).collect() + skills + .into_iter() + .map(|skill| skill.path.to_local_path().unwrap().to_path_buf()) + .collect() } #[test] diff --git a/app/src/ai/skills/resolve_skill_spec.rs b/app/src/ai/skills/resolve_skill_spec.rs index 6b2b743db3..25d2234600 100644 --- a/app/src/ai/skills/resolve_skill_spec.rs +++ b/app/src/ai/skills/resolve_skill_spec.rs @@ -302,7 +302,7 @@ fn resolve_unqualified( let home_matches: Vec = all_matching_paths .iter() .filter(|p| home_skill_paths.contains(p)) - .cloned() + .filter_map(|p| p.to_local_path().map(Path::to_path_buf)) .collect(); if let Some(skill_path) = best_match_by_directory_precedence(home_matches, home_dir.as_deref()) @@ -333,6 +333,7 @@ fn resolve_unqualified( // If we're not in a known repo, try searching across repos under the working directory. let in_scope_matches: Vec = all_matching_paths .into_iter() + .filter_map(|path| path.to_local_path().map(Path::to_path_buf)) .filter(|p| { // Only include project skills (not home skills) that are under working_dir skill_manager.skill_paths_in_scope(working_dir).contains(p) @@ -384,6 +385,7 @@ fn resolve_in_single_repo_root( let cached_paths: Vec = skill_manager .skill_paths_by_name(&spec.skill_identifier) .into_iter() + .filter_map(|path| path.to_local_path().map(Path::to_path_buf)) .filter(|p| repo_skill_paths.contains(p)) .collect(); @@ -454,7 +456,10 @@ fn parsed_skill_from_manager_or_disk( skill_manager: &SkillManager, skill_path: &Path, ) -> Result { - if let Some(parsed) = skill_manager.skill_by_path(skill_path).cloned() { + if let Some(parsed) = skill_manager + .skill_by_path(&LocalOrRemotePath::Local(skill_path.to_path_buf())) + .cloned() + { return Ok(parsed); } diff --git a/app/src/ai/skills/skill_manager.rs b/app/src/ai/skills/skill_manager.rs index 0456b76965..a4275c3b89 100644 --- a/app/src/ai/skills/skill_manager.rs +++ b/app/src/ai/skills/skill_manager.rs @@ -3,10 +3,7 @@ mod file_watchers; use std::collections::{HashMap, HashSet}; use std::path::{Path, PathBuf}; -use ai::skills::{ - get_provider_for_path, parse_bundled_skill, provider_rank, ParsedSkill, SkillProvider, - SkillReference, -}; +use ai::skills::{parse_bundled_skill, provider_rank, ParsedSkill, SkillProvider, SkillReference}; pub use file_watchers::{ extract_skill_parent_directory, read_skills_from_directories, SkillWatcher, SkillWatcherEvent, }; @@ -66,11 +63,11 @@ pub struct SkillManager { /// /// NOT: /// - Key: `/repo/frontend/.agents/skills` - directory_skills: HashMap>, - skills_by_path: HashMap, + directory_skills: HashMap>, + skills_by_path: HashMap, /// Reverse lookup: skill name → set of paths with that name. /// This allows efficient lookup by skill name without scanning all paths. - skills_by_name: HashMap>, + skills_by_name: HashMap>, /// Skills bundled into Warp, each with activation condition and icon. bundled_skills: HashMap, /// When true, all skills in `directory_skills` are in scope regardless of @@ -82,6 +79,27 @@ pub struct SkillManager { skill_watcher: ModelHandle, // Can't remove this or it'll get cleaned up after new() } +pub trait SkillPathQuery { + fn to_skill_location(&self) -> LocalOrRemotePath; +} + +impl SkillPathQuery for LocalOrRemotePath { + fn to_skill_location(&self) -> LocalOrRemotePath { + self.clone() + } +} + +impl SkillPathQuery for Path { + fn to_skill_location(&self) -> LocalOrRemotePath { + LocalOrRemotePath::Local(self.to_path_buf()) + } +} + +impl SkillPathQuery for PathBuf { + fn to_skill_location(&self) -> LocalOrRemotePath { + LocalOrRemotePath::Local(self.clone()) + } +} impl SkillManager { pub fn new(ctx: &mut ModelContext) -> Self { let (skill_watcher_tx, skill_watcher_rx) = async_channel::unbounded(); @@ -125,7 +143,7 @@ impl SkillManager { /// Returns skills available for the given working directory. pub fn get_skills_for_working_directory( &self, - working_directory: Option<&Path>, + working_directory: Option<&LocalOrRemotePath>, ctx: &AppContext, ) -> Vec { // Collect skill paths as (dir_path, skill_path) tuples for later deduplication. @@ -137,7 +155,7 @@ impl SkillManager { skill_paths.extend( self.home_skill_paths() .into_iter() - .map(|path| (home_dir.clone(), path)), + .map(|path| (LocalOrRemotePath::Local(home_dir.clone()), path)), ); } @@ -153,8 +171,7 @@ impl SkillManager { } } else if let Some(working_directory) = working_directory { let repo_root = repo_metadata::repositories::DetectedRepositories::as_ref(ctx) - .get_root_for_path(&LocalOrRemotePath::Local(working_directory.to_path_buf())) - .and_then(|r| PathBuf::try_from(r).ok()); + .get_root_for_path(working_directory); for (dir, dir_skill_paths) in &self.directory_skills { if is_home_directory(dir) { @@ -206,12 +223,12 @@ impl SkillManager { } /// Returns the currently-known home skill file paths. - pub fn home_skill_paths(&self) -> Vec { + pub fn home_skill_paths(&self) -> Vec { let Some(home_dir) = dirs::home_dir() else { return vec![]; }; self.directory_skills - .get(&home_dir) + .get(&LocalOrRemotePath::Local(home_dir)) .map(|skills| skills.iter().cloned().collect()) .unwrap_or_default() } @@ -219,7 +236,11 @@ impl SkillManager { /// Returns the currently-known directories which have skills registered. /// This includes both repo roots and subdirectories with skills. pub fn directories_with_skills(&self) -> Vec { - let mut dirs: Vec = self.directory_skills.keys().cloned().collect(); + let mut dirs: Vec = self + .directory_skills + .keys() + .filter_map(|path| path.to_local_path().map(Path::to_path_buf)) + .collect(); dirs.sort(); dirs } @@ -235,11 +256,16 @@ impl SkillManager { /// Both will be returned. pub fn skill_paths_in_scope(&self, scope_dir: &Path) -> Vec { let mut paths = HashSet::new(); + let scope_dir = LocalOrRemotePath::Local(scope_dir.to_path_buf()); for (dir, skill_paths) in &self.directory_skills { // Include skills from directories that are under scope_dir - if dir.starts_with(scope_dir) { - paths.extend(skill_paths.iter().cloned()); + if dir.starts_with(&scope_dir) { + paths.extend( + skill_paths + .iter() + .filter_map(|path| path.to_local_path().map(Path::to_path_buf)), + ); } } @@ -265,7 +291,7 @@ impl SkillManager { // Slow path: check all paths for this skill name. self.skill_paths_by_name(&skill.name) .iter() - .filter_map(|path| get_provider_for_path(path)) + .filter_map(|path| self.skills_by_path.get(path).map(|skill| skill.provider)) .any(|provider| providers.contains(&provider)) } @@ -292,7 +318,7 @@ impl SkillManager { // Find the supported provider with the best (lowest) rank among all paths. self.skill_paths_by_name(&skill.name) .iter() - .filter_map(|path| get_provider_for_path(path)) + .filter_map(|path| self.skills_by_path.get(path).map(|skill| skill.provider)) .filter(|provider| supported_providers.contains(provider)) .min_by_key(|provider| provider_rank(*provider)) .unwrap_or(skill.provider) @@ -300,25 +326,32 @@ impl SkillManager { /// Returns skill file paths that have the given skill name. /// A skill's name comes from the `name` field in its SKILL.md front matter. - pub fn skill_paths_by_name(&self, name: &str) -> Vec { + pub fn skill_paths_by_name(&self, name: &str) -> Vec { self.skills_by_name .get(name) .map(|paths| { - let mut paths: Vec = paths.iter().cloned().collect(); - paths.sort(); + let mut paths: Vec = paths.iter().cloned().collect(); + paths.sort_by_key(LocalOrRemotePath::display_path); paths }) .unwrap_or_default() } /// Returns a reference to a parsed skill for a specific SKILL.md file path, if it is cached. - pub fn skill_by_path(&self, skill_path: &Path) -> Option<&ParsedSkill> { - self.skills_by_path.get(skill_path) + pub fn skill_by_path( + &self, + skill_path: &P, + ) -> Option<&ParsedSkill> { + self.skills_by_path.get(&skill_path.to_skill_location()) } /// Returns the appropriate `SkillReference` for a skill at the given path. /// For bundled skills, returns `BundledSkillId`; otherwise returns `Path`. - pub fn reference_for_skill_path(&self, skill_path: &Path) -> SkillReference { + pub fn reference_for_skill_path( + &self, + skill_path: &P, + ) -> SkillReference { + let skill_path = skill_path.to_skill_location(); // Check if this path belongs to a bundled skill. for (id, bundled) in &self.bundled_skills { if bundled.skill.path == skill_path { @@ -326,7 +359,7 @@ impl SkillManager { } } // Default to path-based reference. - SkillReference::Path(skill_path.to_path_buf()) + SkillReference::Path(skill_path) } /// Get the definition of a skill, if it is cached. @@ -394,13 +427,13 @@ impl SkillManager { } } - fn handle_skills_deleted(&mut self, paths: Vec) { + fn handle_skills_deleted(&mut self, paths: Vec) { for path in paths { self.handle_path_deleted(&path); } } - fn handle_path_deleted(&mut self, path: &Path) { + fn handle_path_deleted(&mut self, path: &LocalOrRemotePath) { // Delete all skills that are affected by this deleted path for (dir, skill_paths) in &self.directory_skills.clone() { if dir.starts_with(path) { @@ -543,7 +576,7 @@ async fn read_bundled_skills(skills_dir: &Path) -> HashMap let Some(skill_id) = entry_path.file_name().and_then(|s| s.to_str()) else { safe_warn!( safe: ("Could not resolve bundled skill ID, skipping skill"), - full: ("Could not resolve bundled skill ID from {}, skipping skill", skill.path.display()) + full: ("Could not resolve bundled skill ID from {}, skipping skill", skill.path.display_path()) ); continue; }; @@ -628,11 +661,11 @@ fn activation_for_bundled_skill(skill_id: &str, resources_dir: &Path) -> Bundled } } -fn is_home_directory(path: &Path) -> bool { +fn is_home_directory(path: &LocalOrRemotePath) -> bool { let Some(home_dir) = dirs::home_dir() else { return false; }; - path == home_dir + path == &LocalOrRemotePath::Local(home_dir) } impl Entity for SkillManager { diff --git a/app/src/ai/skills/skill_manager_tests.rs b/app/src/ai/skills/skill_manager_tests.rs index 0d036035c6..609a7d8eab 100644 --- a/app/src/ai/skills/skill_manager_tests.rs +++ b/app/src/ai/skills/skill_manager_tests.rs @@ -1,11 +1,14 @@ -use std::collections::{HashMap, HashSet}; -use std::fs; - -use ai::skills::{ParsedSkill, SkillProvider, SkillScope}; +use ai::skills::{get_provider_for_path, ParsedSkill, SkillProvider, SkillReference, SkillScope}; use repo_metadata::repositories::DetectedRepositories; use repo_metadata::{DirectoryWatcher, RepoMetadataModel}; +use std::collections::{HashMap, HashSet}; +use std::fs; use tempfile::TempDir; use warp_core::channel::ChannelState; +use warp_util::{ + host_id::HostId, local_or_remote_path::LocalOrRemotePath, remote_path::RemotePath, + standardized_path::StandardizedPath, +}; use warpui::App; use watcher::HomeDirectoryWatcher; @@ -35,8 +38,9 @@ fn get_skills_for_working_directory_scopes_subdirectory_skills() { fs::create_dir_all(&backend_dir).unwrap(); // Create mock skills - let root_skill_path = repo.join(".agents/skills/root-skill/SKILL.md"); - let frontend_skill_path = frontend_dir.join(".agents/skills/frontend-skill/SKILL.md"); + let root_skill_path = LocalOrRemotePath::Local(repo.join(".agents/skills/root-skill/SKILL.md")); + let frontend_skill_path = + LocalOrRemotePath::Local(frontend_dir.join(".agents/skills/frontend-skill/SKILL.md")); let root_skill = ParsedSkill { name: "root-skill".to_string(), @@ -59,17 +63,18 @@ fn get_skills_for_working_directory_scopes_subdirectory_skills() { }; // Build the internal state manually - let mut directory_skills: HashMap> = HashMap::new(); + let mut directory_skills: HashMap> = + HashMap::new(); directory_skills - .entry(repo.clone()) + .entry(LocalOrRemotePath::Local(repo.clone())) .or_default() .insert(root_skill_path.clone()); directory_skills - .entry(frontend_dir.clone()) + .entry(LocalOrRemotePath::Local(frontend_dir.clone())) .or_default() .insert(frontend_skill_path.clone()); - let mut skills_by_path: HashMap = HashMap::new(); + let mut skills_by_path: HashMap = HashMap::new(); skills_by_path.insert(root_skill_path.clone(), root_skill); skills_by_path.insert(frontend_skill_path.clone(), frontend_skill); @@ -98,7 +103,10 @@ fn get_skills_for_working_directory_scopes_subdirectory_skills() { // Test 1: From frontend directory, should see both root and frontend skills let skills_from_frontend = skill_manager_handle.read(&app, |manager, ctx| { - manager.get_skills_for_working_directory(Some(&frontend_dir), ctx) + manager.get_skills_for_working_directory( + Some(&LocalOrRemotePath::Local(frontend_dir.clone())), + ctx, + ) }); let names_from_frontend: Vec<&str> = skills_from_frontend .iter() @@ -115,7 +123,10 @@ fn get_skills_for_working_directory_scopes_subdirectory_skills() { // Test 2: From backend directory, should only see root skill (not frontend skill) let skills_from_backend = skill_manager_handle.read(&app, |manager, ctx| { - manager.get_skills_for_working_directory(Some(&backend_dir), ctx) + manager.get_skills_for_working_directory( + Some(&LocalOrRemotePath::Local(backend_dir.clone())), + ctx, + ) }); let names_from_backend: Vec<&str> = skills_from_backend .iter() @@ -132,7 +143,10 @@ fn get_skills_for_working_directory_scopes_subdirectory_skills() { // Test 3: From repo root, should only see root skill (not frontend skill) let skills_from_root = skill_manager_handle.read(&app, |manager, ctx| { - manager.get_skills_for_working_directory(Some(&repo), ctx) + manager.get_skills_for_working_directory( + Some(&LocalOrRemotePath::Local(repo.clone())), + ctx, + ) }); let names_from_root: Vec<&str> = skills_from_root.iter().map(|s| s.name.as_str()).collect(); assert!( @@ -159,8 +173,8 @@ fn get_skills_for_working_directory_name_collision_returns_both() { let subdir = repo.join("packages/frontend"); fs::create_dir_all(&subdir).unwrap(); - let root_skill_path = repo.join(".agents/skills/deploy/SKILL.md"); - let subdir_skill_path = subdir.join(".agents/skills/deploy/SKILL.md"); + let root_skill_path = LocalOrRemotePath::Local(repo.join(".agents/skills/deploy/SKILL.md")); + let subdir_skill_path = LocalOrRemotePath::Local(subdir.join(".agents/skills/deploy/SKILL.md")); let root_skill = ParsedSkill { name: "deploy".to_string(), @@ -182,17 +196,18 @@ fn get_skills_for_working_directory_name_collision_returns_both() { scope: SkillScope::Project, }; - let mut directory_skills: HashMap> = HashMap::new(); + let mut directory_skills: HashMap> = + HashMap::new(); directory_skills - .entry(repo.clone()) + .entry(LocalOrRemotePath::Local(repo.clone())) .or_default() .insert(root_skill_path.clone()); directory_skills - .entry(subdir.clone()) + .entry(LocalOrRemotePath::Local(subdir.clone())) .or_default() .insert(subdir_skill_path.clone()); - let mut skills_by_path: HashMap = HashMap::new(); + let mut skills_by_path: HashMap = HashMap::new(); skills_by_path.insert(root_skill_path.clone(), root_skill); skills_by_path.insert(subdir_skill_path.clone(), subdir_skill); @@ -220,7 +235,10 @@ fn get_skills_for_working_directory_name_collision_returns_both() { // From subdir: should see both "deploy" skills (root + subdir) let skills = skill_manager_handle.read(&app, |manager, ctx| { - manager.get_skills_for_working_directory(Some(&subdir), ctx) + manager.get_skills_for_working_directory( + Some(&LocalOrRemotePath::Local(subdir.clone())), + ctx, + ) }); let deploy_skills: Vec<_> = skills.iter().filter(|s| s.name == "deploy").collect(); assert_eq!( @@ -231,7 +249,10 @@ fn get_skills_for_working_directory_name_collision_returns_both() { // From repo root: should only see root "deploy" let skills = skill_manager_handle.read(&app, |manager, ctx| { - manager.get_skills_for_working_directory(Some(&repo), ctx) + manager.get_skills_for_working_directory( + Some(&LocalOrRemotePath::Local(repo.clone())), + ctx, + ) }); let deploy_skills: Vec<_> = skills.iter().filter(|s| s.name == "deploy").collect(); assert_eq!( @@ -256,8 +277,8 @@ fn cloud_environment_skills_always_included() { fs::create_dir_all(&repo_a).unwrap(); fs::create_dir_all(&repo_b).unwrap(); - let skill_a_path = repo_a.join(".agents/skills/build/SKILL.md"); - let skill_b_path = repo_b.join(".agents/skills/deploy/SKILL.md"); + let skill_a_path = LocalOrRemotePath::Local(repo_a.join(".agents/skills/build/SKILL.md")); + let skill_b_path = LocalOrRemotePath::Local(repo_b.join(".agents/skills/deploy/SKILL.md")); let skill_a = ParsedSkill { name: "build".to_string(), @@ -279,17 +300,18 @@ fn cloud_environment_skills_always_included() { scope: SkillScope::Project, }; - let mut directory_skills: HashMap> = HashMap::new(); + let mut directory_skills: HashMap> = + HashMap::new(); directory_skills - .entry(repo_a.clone()) + .entry(LocalOrRemotePath::Local(repo_a.clone())) .or_default() .insert(skill_a_path.clone()); directory_skills - .entry(repo_b.clone()) + .entry(LocalOrRemotePath::Local(repo_b.clone())) .or_default() .insert(skill_b_path.clone()); - let mut skills_by_path: HashMap = HashMap::new(); + let mut skills_by_path: HashMap = HashMap::new(); skills_by_path.insert(skill_a_path.clone(), skill_a); skills_by_path.insert(skill_b_path.clone(), skill_b); @@ -318,7 +340,10 @@ fn cloud_environment_skills_always_included() { // From inside repo_a, both repo_a and repo_b skills are visible // because is_cloud_environment skips the ancestor filter. let skills = skill_manager_handle.read(&app, |manager, ctx| { - manager.get_skills_for_working_directory(Some(&repo_a), ctx) + manager.get_skills_for_working_directory( + Some(&LocalOrRemotePath::Local(repo_a.clone())), + ctx, + ) }); let names: Vec<&str> = skills.iter().map(|s| s.name.as_str()).collect(); assert!( @@ -478,23 +503,101 @@ fn test_build_bundled_skill_context() { ); } +fn make_remote_skill(host_id: &HostId, name: &str) -> ParsedSkill { + ParsedSkill { + name: name.to_string(), + description: format!("{name} remote skill"), + path: LocalOrRemotePath::Remote(RemotePath::new( + host_id.clone(), + StandardizedPath::try_new(format!("/repo/.agents/skills/{name}/SKILL.md").as_str()) + .unwrap(), + )), + content: format!("# {name}"), + line_range: None, + provider: SkillProvider::Agents, + scope: SkillScope::Project, + } +} + +#[test] +fn active_skill_by_reference_resolves_exact_remote_identity() { + let remote_skill = make_remote_skill(&HostId::new("remote-host".to_string()), "deploy"); + let reference = SkillReference::Path(remote_skill.path.clone()); + + App::test((), |mut app| async move { + app.add_singleton_model(DirectoryWatcher::new); + app.add_singleton_model(AISettings::new_with_defaults); + app.add_singleton_model(|_| DetectedRepositories::default()); + app.add_singleton_model(RepoMetadataModel::new); + app.add_singleton_model(HomeDirectoryWatcher::new_for_test); + app.add_singleton_model(WarpManagedPathsWatcher::new_for_testing); + let handle = app.add_singleton_model(SkillManager::new); + + handle.update(&mut app, |manager, _| { + manager.add_skill_for_testing(remote_skill.clone()); + }); + + let resolved = handle.read(&app, |manager, ctx| { + manager + .active_skill_by_reference(&reference, ctx) + .map(|skill| skill.path.clone()) + }); + + assert_eq!(resolved, Some(remote_skill.path)); + }); +} + +#[test] +fn active_skill_by_reference_distinguishes_remote_hosts_with_the_same_display_path() { + let first_skill = make_remote_skill(&HostId::new("first-host".to_string()), "deploy"); + let second_skill = make_remote_skill(&HostId::new("second-host".to_string()), "deploy"); + let first_path = first_skill.path.clone(); + let second_path = second_skill.path.clone(); + let first_reference = SkillReference::Path(first_skill.path.clone()); + let second_reference = SkillReference::Path(second_skill.path.clone()); + + App::test((), |mut app| async move { + app.add_singleton_model(DirectoryWatcher::new); + app.add_singleton_model(AISettings::new_with_defaults); + app.add_singleton_model(|_| DetectedRepositories::default()); + app.add_singleton_model(RepoMetadataModel::new); + app.add_singleton_model(HomeDirectoryWatcher::new_for_test); + app.add_singleton_model(WarpManagedPathsWatcher::new_for_testing); + let handle = app.add_singleton_model(SkillManager::new); + + handle.update(&mut app, |manager, _| { + manager.add_skill_for_testing(first_skill); + manager.add_skill_for_testing(second_skill); + }); + + let resolved = handle.read(&app, |manager, ctx| { + ( + manager + .active_skill_by_reference(&first_reference, ctx) + .map(|skill| skill.path.clone()), + manager + .active_skill_by_reference(&second_reference, ctx) + .map(|skill| skill.path.clone()), + ) + }); + assert_eq!(resolved, (Some(first_path), Some(second_path))); + }); +} + // ============================================================================ // Tests for best_supported_provider // ============================================================================ /// Helper: creates a ParsedSkill under a given provider directory. fn make_skill(name: &str, provider_dir: &str) -> ParsedSkill { - let path = PathBuf::from(format!("/repo/{provider_dir}/skills/{name}/SKILL.md")); + let local_path = PathBuf::from(format!("/repo/{provider_dir}/skills/{name}/SKILL.md")); ParsedSkill { name: name.to_string(), description: format!("{name} skill"), - path, + path: LocalOrRemotePath::Local(local_path.clone()), content: format!("# {name}"), line_range: None, - provider: get_provider_for_path(&PathBuf::from(format!( - "/repo/{provider_dir}/skills/{name}/SKILL.md" - ))) - .unwrap_or(SkillProvider::Warp), + provider: get_provider_for_path(&local_path).unwrap_or(SkillProvider::Warp), scope: SkillScope::Project, } } diff --git a/app/src/ai/skills/skill_utils.rs b/app/src/ai/skills/skill_utils.rs index 5abe54f0e7..6ae75eb132 100644 --- a/app/src/ai/skills/skill_utils.rs +++ b/app/src/ai/skills/skill_utils.rs @@ -13,6 +13,7 @@ use siphasher::sip::SipHasher; use warp_core::ui::appearance::Appearance; use warp_core::ui::theme::color::internal_colors; use warp_core::ui::Icon; +use warp_util::local_or_remote_path::LocalOrRemotePath; use warpui::prelude::MouseStateHandle; use warpui::{AppContext, Element, EventContext, SingletonEntity}; @@ -32,7 +33,7 @@ lazy_static! { fn try_insert_skill( dedup_map: &mut HashMap, descriptor: SkillDescriptor, - dir_path: &Path, + dir_path: &LocalOrRemotePath, content: &str, ) { let mut hasher = *CONTENT_HASHER; @@ -65,8 +66,8 @@ fn try_insert_skill( /// `dir_path` is the directory that owns the skill. #[cfg_attr(not(feature = "local_fs"), allow(dead_code))] pub(crate) fn unique_skills( - skill_paths: &[(PathBuf, PathBuf)], - skills_by_path: &HashMap, + skill_paths: &[(LocalOrRemotePath, LocalOrRemotePath)], + skills_by_path: &HashMap, ) -> Vec { // hash(dir_path + content) → best descriptor seen so far let mut dedup_map: HashMap = HashMap::new(); @@ -88,7 +89,7 @@ pub(crate) fn unique_skills( /// Returns the list of skills if they have changed since the last time we sent them to the server. /// Skills are always included except when the current list matches the last list sent. pub fn list_skills_if_changed( - working_directory: Option<&Path>, + working_directory: Option<&LocalOrRemotePath>, conversation_id: Option, app: &AppContext, ) -> Option> { diff --git a/app/src/ai/skills/skill_utils_tests.rs b/app/src/ai/skills/skill_utils_tests.rs index d8ac47b8bb..52c2261437 100644 --- a/app/src/ai/skills/skill_utils_tests.rs +++ b/app/src/ai/skills/skill_utils_tests.rs @@ -1,4 +1,5 @@ use std::path::PathBuf; +use warp_util::local_or_remote_path::LocalOrRemotePath; use ai::skills::{ParsedSkill, SkillProvider, SkillScope}; @@ -64,7 +65,7 @@ fn test_unique_skills_dedupes_identical_skills_same_dir() { let content = "---\nname: test-skill\ndescription: A test skill\n---\nContent here"; let skill = ParsedSkill { - path: skill_path1.clone(), + path: LocalOrRemotePath::Local(skill_path1.clone()), name: "test-skill".to_string(), description: "A test skill".to_string(), content: content.to_string(), @@ -74,7 +75,7 @@ fn test_unique_skills_dedupes_identical_skills_same_dir() { }; let skill2 = ParsedSkill { - path: skill_path2.clone(), + path: LocalOrRemotePath::Local(skill_path2.clone()), name: "test-skill".to_string(), description: "A test skill".to_string(), content: content.to_string(), @@ -84,12 +85,18 @@ fn test_unique_skills_dedupes_identical_skills_same_dir() { }; let mut skills_by_path = HashMap::new(); - skills_by_path.insert(skill_path1.clone(), skill); - skills_by_path.insert(skill_path2.clone(), skill2); + skills_by_path.insert(LocalOrRemotePath::Local(skill_path1.clone()), skill); + skills_by_path.insert(LocalOrRemotePath::Local(skill_path2.clone()), skill2); let skill_paths = vec![ - (shared_skill_dir.clone(), skill_path1), - (shared_skill_dir, skill_path2), + ( + LocalOrRemotePath::Local(shared_skill_dir.clone()), + LocalOrRemotePath::Local(skill_path1), + ), + ( + LocalOrRemotePath::Local(shared_skill_dir), + LocalOrRemotePath::Local(skill_path2), + ), ]; let result = unique_skills(&skill_paths, &skills_by_path); @@ -107,7 +114,7 @@ fn test_unique_skills_does_not_dedupe_different_dirs() { let content = "---\nname: test-skill\ndescription: A test skill\n---\nContent here"; let home_skill = ParsedSkill { - path: home_path.clone(), + path: LocalOrRemotePath::Local(home_path.clone()), name: "test-skill".to_string(), description: "A test skill".to_string(), content: content.to_string(), @@ -117,7 +124,7 @@ fn test_unique_skills_does_not_dedupe_different_dirs() { }; let project_skill = ParsedSkill { - path: project_path.clone(), + path: LocalOrRemotePath::Local(project_path.clone()), name: "test-skill".to_string(), description: "A test skill".to_string(), content: content.to_string(), @@ -127,10 +134,22 @@ fn test_unique_skills_does_not_dedupe_different_dirs() { }; let mut skills_by_path = HashMap::new(); - skills_by_path.insert(home_path.clone(), home_skill); - skills_by_path.insert(project_path.clone(), project_skill); + skills_by_path.insert(LocalOrRemotePath::Local(home_path.clone()), home_skill); + skills_by_path.insert( + LocalOrRemotePath::Local(project_path.clone()), + project_skill, + ); - let skill_paths = vec![(home_dir, home_path), (project_dir, project_path)]; + let skill_paths = vec![ + ( + LocalOrRemotePath::Local(home_dir), + LocalOrRemotePath::Local(home_path), + ), + ( + LocalOrRemotePath::Local(project_dir), + LocalOrRemotePath::Local(project_path), + ), + ]; let result = unique_skills(&skill_paths, &skills_by_path); assert_eq!( @@ -150,7 +169,7 @@ fn test_unique_skills_does_not_dedupe_different_content() { let content2 = "---\nname: test-skill\ndescription: A test skill\n---\nDifferent content"; let skill1 = ParsedSkill { - path: skill_path1.clone(), + path: LocalOrRemotePath::Local(skill_path1.clone()), name: "test-skill".to_string(), description: "A test skill".to_string(), content: content1.to_string(), @@ -160,7 +179,7 @@ fn test_unique_skills_does_not_dedupe_different_content() { }; let skill2 = ParsedSkill { - path: skill_path2.clone(), + path: LocalOrRemotePath::Local(skill_path2.clone()), name: "test-skill".to_string(), description: "A test skill".to_string(), content: content2.to_string(), @@ -170,12 +189,18 @@ fn test_unique_skills_does_not_dedupe_different_content() { }; let mut skills_by_path = HashMap::new(); - skills_by_path.insert(skill_path1.clone(), skill1); - skills_by_path.insert(skill_path2.clone(), skill2); + skills_by_path.insert(LocalOrRemotePath::Local(skill_path1.clone()), skill1); + skills_by_path.insert(LocalOrRemotePath::Local(skill_path2.clone()), skill2); let skill_paths = vec![ - (shared_skill_dir.clone(), skill_path1), - (shared_skill_dir, skill_path2), + ( + LocalOrRemotePath::Local(shared_skill_dir.clone()), + LocalOrRemotePath::Local(skill_path1), + ), + ( + LocalOrRemotePath::Local(shared_skill_dir), + LocalOrRemotePath::Local(skill_path2), + ), ]; let result = unique_skills(&skill_paths, &skills_by_path); diff --git a/app/src/code/editor_management.rs b/app/src/code/editor_management.rs index 3988737d1d..0832be6f38 100644 --- a/app/src/code/editor_management.rs +++ b/app/src/code/editor_management.rs @@ -1,6 +1,6 @@ use std::collections::hash_map::Entry; use std::collections::HashMap; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use ai::skills::SkillReference; use serde::{Deserialize, Serialize}; @@ -114,7 +114,7 @@ pub enum CodeSource { /// Opened from an active AI agent conversation. AIAction { id: AIAgentActionId }, /// Opened from project rules (WARP.md) file. - ProjectRules { path: PathBuf }, + ProjectRules { location: LocalOrRemotePath }, /// Opened from file tree (local or remote). FileTree { location: LocalOrRemotePath }, /// Opened from command palette file search (local or remote). @@ -124,7 +124,7 @@ pub enum CodeSource { /// Opened from a skill. Skill { reference: SkillReference, - path: PathBuf, + location: LocalOrRemotePath, origin: SkillOpenOrigin, }, } @@ -154,10 +154,10 @@ impl CodeSource { LocalOrRemotePath::Remote(_) => None, } } - Self::Link { path, .. } - | Self::ProjectRules { path } - | Self::Finder { path } - | Self::Skill { path, .. } => Some(path.clone()), + Self::Link { path, .. } | Self::Finder { path } => Some(path.clone()), + Self::ProjectRules { location } | Self::Skill { location, .. } => { + location.to_local_path().map(Path::to_path_buf) + } } } @@ -180,10 +180,12 @@ impl CodeSource { Self::FileTree { location } | Self::CommandPalette { location } => { Some(location.clone()) } - Self::Link { path, .. } - | Self::ProjectRules { path } - | Self::Finder { path } - | Self::Skill { path, .. } => Some(LocalOrRemotePath::Local(path.clone())), + Self::Link { path, .. } | Self::Finder { path } => { + Some(LocalOrRemotePath::Local(path.clone())) + } + Self::ProjectRules { location } | Self::Skill { location, .. } => { + Some(location.clone()) + } } } @@ -244,6 +246,13 @@ impl CodeSource { | Self::CommandPalette { location: LocalOrRemotePath::Remote(_), } + | Self::ProjectRules { + location: LocalOrRemotePath::Remote(_), + } + | Self::Skill { + location: LocalOrRemotePath::Remote(_), + .. + } ) } } diff --git a/app/src/search/ai_context_menu/skills/data_source.rs b/app/src/search/ai_context_menu/skills/data_source.rs index 6f99241bab..73a6aa7f31 100644 --- a/app/src/search/ai_context_menu/skills/data_source.rs +++ b/app/src/search/ai_context_menu/skills/data_source.rs @@ -1,6 +1,7 @@ use std::path::PathBuf; use fuzzy_match::FuzzyMatchResult; +use warp_util::local_or_remote_path::LocalOrRemotePath; use warpui::{AppContext, Entity, SingletonEntity}; use super::search_item::SkillSearchItem; @@ -47,8 +48,8 @@ impl SyncDataSource for SkillsDataSource { } }; - let skills = - SkillManager::as_ref(app).get_skills_for_working_directory(cwd.as_deref(), app); + let cwd = cwd.map(LocalOrRemotePath::Local); + let skills = SkillManager::as_ref(app).get_skills_for_working_directory(cwd.as_ref(), app); let mut results: Vec> = if query_text.is_empty() { // Zero state: show all skills with a uniform high score. diff --git a/app/src/terminal/input/skills/data_source.rs b/app/src/terminal/input/skills/data_source.rs index bde833aa4f..5590974a84 100644 --- a/app/src/terminal/input/skills/data_source.rs +++ b/app/src/terminal/input/skills/data_source.rs @@ -5,6 +5,7 @@ use fuzzy_match::{match_indices_case_insensitive, FuzzyMatchResult}; use ordered_float::OrderedFloat; use warp_core::ui::icons::Icon; use warp_core::ui::theme::Fill; +use warp_util::local_or_remote_path::LocalOrRemotePath; use warpui::elements::{ ConstrainedBox, Container, CrossAxisAlignment, Flex, Highlight, ParentElement, Shrinkable, Text, }; @@ -123,8 +124,8 @@ impl SyncDataSource for SkillSelectorDataSource { ) -> Result>, DataSourceRunErrorWrapper> { let cwd = self.get_current_working_directory(app); let cli_agent_providers = self.active_cli_agent_providers(app); - let skills = - SkillManager::as_ref(app).get_skills_for_working_directory(cwd.as_deref(), app); + let cwd = cwd.map(LocalOrRemotePath::Local); + let skills = SkillManager::as_ref(app).get_skills_for_working_directory(cwd.as_ref(), app); // Filter out bundled skills when in open mode, since they cannot be opened. // When CLI agent input is open, filter to skills that exist in a supported diff --git a/app/src/terminal/input/slash_command_model.rs b/app/src/terminal/input/slash_command_model.rs index 6171b7fb11..438e1894f1 100644 --- a/app/src/terminal/input/slash_command_model.rs +++ b/app/src/terminal/input/slash_command_model.rs @@ -1,7 +1,9 @@ use ai::skills::SkillReference; use input_classifier::InputType; use settings::Setting as _; +use std::path::PathBuf; use warp_core::features::FeatureFlag; +use warp_util::local_or_remote_path::LocalOrRemotePath; use warpui::{AppContext, Entity, ModelContext, ModelHandle, SingletonEntity}; use crate::ai::blocklist::{ @@ -248,11 +250,14 @@ impl SlashCommandModel { let skill_name = possible_command.strip_prefix('/')?; - let cwd = self.active_session.as_ref(ctx).current_working_directory(); - let cwd_path = cwd.as_ref().map(std::path::Path::new); + let cwd = self + .active_session + .as_ref(ctx) + .current_working_directory() + .map(|cwd| LocalOrRemotePath::Local(PathBuf::from(cwd))); let skills = SkillManager::handle(ctx) .as_ref(ctx) - .get_skills_for_working_directory(cwd_path, ctx); + .get_skills_for_working_directory(cwd.as_ref(), ctx); let matched_skill = skills.into_iter().find(|skill| skill.name == skill_name)?; diff --git a/app/src/terminal/input/slash_commands/data_source/mod.rs b/app/src/terminal/input/slash_commands/data_source/mod.rs index 42566acfba..10bcdc801e 100644 --- a/app/src/terminal/input/slash_commands/data_source/mod.rs +++ b/app/src/terminal/input/slash_commands/data_source/mod.rs @@ -13,6 +13,7 @@ use warp_cli::agent::Harness; use warp_core::features::FeatureFlag; use warp_core::ui::appearance::Appearance; use warp_core::ui::Icon as WarpIcon; +use warp_util::local_or_remote_path::LocalOrRemotePath; use warpui::fonts::FamilyId; use warpui::{AppContext, Entity, EntityId, ModelContext, ModelHandle, SingletonEntity}; pub use zero_state::*; @@ -504,11 +505,14 @@ impl SyncDataSource for SlashCommandDataSource { // Skills are invoked by the agent, so they're hidden entirely when AI is globally off. if FeatureFlag::ListSkills.is_enabled() && AISettings::as_ref(app).is_any_ai_enabled(app) { let cli_agent_providers = self.active_cli_agent_providers(app); - let cwd = self.active_session.as_ref(app).current_working_directory(); - let cwd_path = cwd.as_ref().map(std::path::Path::new); + let cwd = self + .active_session + .as_ref(app) + .current_working_directory() + .map(|cwd| LocalOrRemotePath::Local(cwd.into())); let skills = SkillManager::handle(app) .as_ref(app) - .get_skills_for_working_directory(cwd_path, app); + .get_skills_for_working_directory(cwd.as_ref(), app); let skill_manager = SkillManager::as_ref(app); for mut skill in skills { diff --git a/app/src/terminal/input/slash_commands/data_source/zero_state.rs b/app/src/terminal/input/slash_commands/data_source/zero_state.rs index 42901ab14d..112d6acc1d 100644 --- a/app/src/terminal/input/slash_commands/data_source/zero_state.rs +++ b/app/src/terminal/input/slash_commands/data_source/zero_state.rs @@ -1,5 +1,6 @@ use itertools::Itertools; use warp_core::features::FeatureFlag; +use warp_util::local_or_remote_path::LocalOrRemotePath; use warpui::{Entity, ModelHandle, SingletonEntity}; use crate::ai::skills::SkillManager; @@ -108,11 +109,11 @@ impl SyncDataSource for ZeroStateDataSource { let cwd = slash_command_data_source .active_session_for_v2_zero_state() .as_ref(app) - .current_working_directory(); - let cwd_path = cwd.as_ref().map(std::path::Path::new); + .current_working_directory() + .map(|cwd| LocalOrRemotePath::Local(cwd.into())); let skill_manager_handle = SkillManager::handle(app); let skill_manager = skill_manager_handle.as_ref(app); - let skills = skill_manager.get_skills_for_working_directory(cwd_path, app); + let skills = skill_manager.get_skills_for_working_directory(cwd.as_ref(), app); for mut skill in skills .into_iter() diff --git a/app/src/terminal/view.rs b/app/src/terminal/view.rs index 02b925e1b4..98fa329739 100644 --- a/app/src/terminal/view.rs +++ b/app/src/terminal/view.rs @@ -26557,7 +26557,9 @@ impl TypedActionView for TerminalView { warp_md_path.push(WARP_MD_PATH); #[cfg(feature = "local_fs")] ctx.emit(Event::OpenCodeInWarp { - source: CodeSource::ProjectRules { path: warp_md_path }, + source: CodeSource::ProjectRules { + location: LocalOrRemotePath::Local(warp_md_path), + }, layout: *crate::util::file::external_editor::EditorSettings::as_ref(ctx) .open_file_layout .value(), @@ -26593,7 +26595,7 @@ impl TypedActionView for TerminalView { ctx.emit(Event::OpenCodeInWarp { source: CodeSource::Skill { reference: skill_reference.clone(), - path: path.clone(), + location: path.clone(), origin: SkillOpenOrigin::OpenSkillCommand, }, layout: diff --git a/app/src/workspace/view.rs b/app/src/workspace/view.rs index 2dcdfe5cd2..b831d18e32 100644 --- a/app/src/workspace/view.rs +++ b/app/src/workspace/view.rs @@ -13282,7 +13282,7 @@ impl Workspace { if let Some((first, rest)) = rule_paths.split_first() { self.open_code( CodeSource::ProjectRules { - path: first.clone(), + location: LocalOrRemotePath::Local(first.clone()), }, EditorLayout::SplitPane, None, diff --git a/crates/ai/src/agent/action/convert.rs b/crates/ai/src/agent/action/convert.rs index 074789d461..ec53172d9d 100644 --- a/crates/ai/src/agent/action/convert.rs +++ b/crates/ai/src/agent/action/convert.rs @@ -8,16 +8,14 @@ use warp_multi_agent_api as api; use crate::agent::action::{ AIAgentActionType, AIAgentPtyWriteMode, CommentSide, FileEdit, InsertReviewComment, - InsertedCommentLine, InsertedCommentLocation, ReadFilesRequest, ReadSkillRequest, - SearchCodebaseRequest, ShellCommandDelay, SuggestPromptRequest, UploadArtifactRequest, - UseComputerRequest, + InsertedCommentLine, InsertedCommentLocation, ReadFilesRequest, SearchCodebaseRequest, + ShellCommandDelay, SuggestPromptRequest, UploadArtifactRequest, UseComputerRequest, }; use crate::agent::action_result::{AnyFileContent, FileContext}; use crate::agent::convert::ToolToAIAgentActionError; use crate::agent::FileLocations; use crate::diff_validation::{ParsedDiff, V4AHunk}; use crate::document::AIDocumentId; -use crate::skills::SkillReference; impl From for AIAgentActionType { fn from(value: api::message::tool_call::RunShellCommand) -> Self { @@ -502,19 +500,6 @@ impl From for AIAgentActionType { } } -impl TryFrom for AIAgentActionType { - type Error = ToolToAIAgentActionError; - - fn try_from(value: api::message::tool_call::ReadSkill) -> Result { - match value.skill_reference { - Some(reference) => Ok(AIAgentActionType::ReadSkill(ReadSkillRequest { - skill: SkillReference::from(reference), - })), - None => Err(ToolToAIAgentActionError::MissingSkillReference), - } - } -} - impl From for AIAgentActionType { fn from(value: api::message::tool_call::FetchConversation) -> Self { AIAgentActionType::FetchConversation { @@ -523,18 +508,6 @@ impl From for AIAgentActionType { } } -impl From for SkillReference { - fn from(value: api::message::tool_call::read_skill::SkillReference) -> Self { - use warp_multi_agent_api::message::tool_call::read_skill::SkillReference as ApiSkillReference; - match value { - ApiSkillReference::SkillPath(skill_path) => { - SkillReference::Path(PathBuf::from(skill_path)) - } - ApiSkillReference::BundledSkillId(id) => SkillReference::BundledSkillId(id), - } - } -} - /// Converts API ScreenshotParams to the internal computer_use type. fn convert_screenshot_params( params: api::message::tool_call::ScreenshotParams, diff --git a/crates/ai/src/skills/conversion.rs b/crates/ai/src/skills/conversion.rs index 05483d8147..37dd63ec1a 100644 --- a/crates/ai/src/skills/conversion.rs +++ b/crates/ai/src/skills/conversion.rs @@ -1,10 +1,13 @@ use std::path::PathBuf; - use thiserror::Error; use warp_multi_agent_api as api; +use warp_util::host_id::HostId; +use warp_util::local_or_remote_path::LocalOrRemotePath; +use warp_util::remote_path::RemotePath; +use warp_util::standardized_path::StandardizedPath; use crate::agent::action_result::{AnyFileContent, FileContext}; -use crate::skills::{ParsedSkill, SkillProvider, SkillScope}; +use crate::skills::{ParsedSkill, SkillProvider, SkillReference, SkillScope}; #[derive(Error, Debug)] pub enum SkillConversionError { @@ -20,14 +23,98 @@ pub enum SkillConversionError { ProviderInvalid, #[error("Invalid content")] ContentInvalid, + #[error("Skill path origin is unavailable")] + PathOriginUnavailable, + #[error("Invalid remote skill path")] + RemotePathInvalid, +} +/// Identifies how a string skill path from an API payload should be interpreted. +/// +/// Live agent responses can be decoded from the active session's location. Restored payloads do +/// not carry enough session identity to safely reconstruct path-based skill locations, so callers +/// must use [`SkillPathOrigin::Unavailable`] rather than silently assuming the local filesystem. +#[derive(Debug, Clone, Eq, PartialEq)] +pub enum SkillPathOrigin { + Local, + Remote { + host_id: HostId, + }, + /// Path identity could not be restored, but the API payload already carries the skill + /// descriptor and content needed to render a historical transcript. + /// + /// This intentionally uses a local path wrapper only as a display-compatible identity for + /// restored conversation UI. Live execution paths should use [`SkillPathOrigin::Local`] or + /// [`SkillPathOrigin::Remote`] so local/remote provenance is preserved. + RestoredDisplayOnly, + Unavailable, +} + +impl SkillPathOrigin { + pub fn location_for_path( + &self, + path: impl Into, + ) -> Result { + let path = path.into(); + match self { + SkillPathOrigin::Local | SkillPathOrigin::RestoredDisplayOnly => { + Ok(LocalOrRemotePath::Local(PathBuf::from(path))) + } + SkillPathOrigin::Remote { host_id } => { + let path = StandardizedPath::try_new(&path) + .map_err(|_| SkillConversionError::RemotePathInvalid)?; + Ok(LocalOrRemotePath::Remote(RemotePath::new( + host_id.clone(), + path, + ))) + } + SkillPathOrigin::Unavailable => Err(SkillConversionError::PathOriginUnavailable), + } + } } +fn skill_reference_for_path( + path: impl Into, + path_origin: &SkillPathOrigin, +) -> Result { + path_origin + .location_for_path(path) + .map(SkillReference::Path) +} + +pub fn skill_reference_from_api_skill_ref( + skill_ref: api::SkillRef, + path_origin: &SkillPathOrigin, +) -> Option { + match skill_ref.skill_reference { + Some(api::skill_ref::SkillReference::Path(path)) => { + skill_reference_for_path(path, path_origin).ok() + } + Some(api::skill_ref::SkillReference::BundledSkillId(id)) => { + Some(SkillReference::BundledSkillId(id)) + } + None => None, + } +} + +pub fn skill_reference_from_read_skill_ref( + skill_reference: api::message::tool_call::read_skill::SkillReference, + path_origin: &SkillPathOrigin, +) -> Result { + match skill_reference { + api::message::tool_call::read_skill::SkillReference::SkillPath(path) => { + skill_reference_for_path(path, path_origin) + } + api::message::tool_call::read_skill::SkillReference::BundledSkillId(id) => { + Ok(SkillReference::BundledSkillId(id)) + } + } +} impl From for api::Skill { fn from(skill: ParsedSkill) -> Self { api::Skill { descriptor: Some(api::SkillDescriptor { skill_reference: Some(api::skill_descriptor::SkillReference::Path( - skill.path.to_string_lossy().to_string(), + skill.path.display_path(), )), name: skill.name, description: skill.description, @@ -35,7 +122,7 @@ impl From for api::Skill { provider: Some(skill.provider.into()), }), content: Some(api::FileContent { - file_path: skill.path.to_string_lossy().to_string(), + file_path: skill.path.display_path(), content: skill.content, line_range: skill .line_range @@ -87,6 +174,15 @@ impl TryFrom for ParsedSkill { type Error = SkillConversionError; fn try_from(api_skill: api::Skill) -> Result { + Self::try_from_api_with_origin(api_skill, &SkillPathOrigin::Unavailable) + } +} + +impl ParsedSkill { + pub fn try_from_api_with_origin( + api_skill: api::Skill, + path_origin: &SkillPathOrigin, + ) -> Result { let Some(descriptor) = api_skill.descriptor else { return Err(SkillConversionError::MissingDescriptor); }; @@ -119,7 +215,7 @@ impl TryFrom for ParsedSkill { let line_range = context.line_range.as_ref(); Ok(ParsedSkill { - path: PathBuf::from(&path), + path: path_origin.location_for_path(path)?, name: descriptor.name, description: descriptor.description, content, @@ -162,3 +258,7 @@ fn convert_provider( api::skill_descriptor::provider::Type::OpenCode(_) => Ok(SkillProvider::OpenCode), } } + +#[cfg(test)] +#[path = "conversion_tests.rs"] +mod conversion_tests; diff --git a/crates/ai/src/skills/conversion_tests.rs b/crates/ai/src/skills/conversion_tests.rs new file mode 100644 index 0000000000..2c95549565 --- /dev/null +++ b/crates/ai/src/skills/conversion_tests.rs @@ -0,0 +1,100 @@ +use warp_multi_agent_api as api; +use warp_util::host_id::HostId; +use warp_util::local_or_remote_path::LocalOrRemotePath; + +use super::{ + skill_reference_from_api_skill_ref, skill_reference_from_read_skill_ref, SkillConversionError, + SkillPathOrigin, +}; +use crate::skills::{ParsedSkill, SkillReference}; + +fn api_project_skill(path: &str) -> api::Skill { + api::Skill { + descriptor: Some(api::SkillDescriptor { + skill_reference: Some(api::skill_descriptor::SkillReference::Path( + path.to_string(), + )), + name: "deploy".to_string(), + description: "Deploy the service".to_string(), + scope: Some(api::skill_descriptor::Scope { + r#type: Some(api::skill_descriptor::scope::Type::Project(())), + }), + provider: Some(api::skill_descriptor::Provider { + r#type: Some(api::skill_descriptor::provider::Type::Agents(())), + }), + }), + content: Some(api::FileContent { + file_path: path.to_string(), + content: "# Deploy".to_string(), + line_range: None, + }), + } +} + +#[test] +fn try_from_api_with_remote_origin_preserves_host_identity() { + let host_id = HostId::new("remote-host".to_string()); + let parsed = ParsedSkill::try_from_api_with_origin( + api_project_skill("/repo/.agents/skills/deploy/SKILL.md"), + &SkillPathOrigin::Remote { + host_id: host_id.clone(), + }, + ) + .expect("remote project skill should convert"); + + let LocalOrRemotePath::Remote(path) = parsed.path else { + panic!("expected a remote skill path"); + }; + assert_eq!(path.host_id, host_id); + assert_eq!(path.path.as_str(), "/repo/.agents/skills/deploy/SKILL.md"); +} + +#[test] +fn try_from_api_with_unavailable_origin_rejects_path_based_skills() { + let error = ParsedSkill::try_from_api_with_origin( + api_project_skill("/repo/.agents/skills/deploy/SKILL.md"), + &SkillPathOrigin::Unavailable, + ) + .expect_err("restored skills without host context should not fabricate local paths"); + + assert!(matches!(error, SkillConversionError::PathOriginUnavailable)); +} + +#[test] +fn skill_ref_with_unavailable_origin_preserves_bundled_skills() { + let skill_reference = skill_reference_from_api_skill_ref( + api::SkillRef { + skill_reference: Some(api::skill_ref::SkillReference::BundledSkillId( + "review-comments".to_string(), + )), + }, + &SkillPathOrigin::Unavailable, + ); + + assert_eq!( + skill_reference, + Some(SkillReference::BundledSkillId( + "review-comments".to_string() + )) + ); +} + +#[test] +fn read_skill_ref_with_remote_origin_preserves_host_identity() { + let host_id = HostId::new("remote-host".to_string()); + let skill_reference = skill_reference_from_read_skill_ref( + api::message::tool_call::read_skill::SkillReference::SkillPath( + "/repo/.agents/skills/deploy/SKILL.md".to_string(), + ), + &SkillPathOrigin::Remote { + host_id: host_id.clone(), + }, + ) + .expect("remote read_skill skill references should convert"); + + let SkillReference::Path(LocalOrRemotePath::Remote(path)) = skill_reference else { + panic!("expected a remote skill path"); + }; + assert_eq!(path.host_id, host_id); + assert_eq!(path.path.as_str(), "/repo/.agents/skills/deploy/SKILL.md"); +} diff --git a/crates/ai/src/skills/mod.rs b/crates/ai/src/skills/mod.rs index 1f9d729ce0..409e87dd80 100644 --- a/crates/ai/src/skills/mod.rs +++ b/crates/ai/src/skills/mod.rs @@ -4,8 +4,14 @@ mod parser; mod read_skills; mod skill_provider; mod skill_reference; +pub use conversion::{ + skill_reference_from_api_skill_ref, skill_reference_from_read_skill_ref, SkillConversionError, + SkillPathOrigin, +}; -pub use parse_skill::{parse_bundled_skill, parse_skill, ParsedSkill}; +pub use parse_skill::{ + parse_bundled_skill, parse_skill, parse_skill_content_at_location, ParsedSkill, +}; pub use read_skills::read_skills; pub use skill_provider::{ get_provider_for_path, home_skills_path, provider_rank, SkillProvider, SkillProviderDefinition, diff --git a/crates/ai/src/skills/parse_skill.rs b/crates/ai/src/skills/parse_skill.rs index dbf37325d0..dd5c3a9aa7 100644 --- a/crates/ai/src/skills/parse_skill.rs +++ b/crates/ai/src/skills/parse_skill.rs @@ -1,14 +1,15 @@ use std::fmt::Display; +use std::fs; use std::ops::Range; -use std::path::{Path, PathBuf}; +use std::path::Path; +use super::parser::parse_markdown_content; +use super::skill_provider::{get_provider_for_path, get_scope_for_path, SkillProvider, SkillScope}; use anyhow::Result; use lazy_static::lazy_static; use regex::Regex; use thiserror::Error; - -use super::parser::parse_markdown_file; -use super::skill_provider::{get_provider_for_path, get_scope_for_path, SkillProvider, SkillScope}; +use warp_util::local_or_remote_path::LocalOrRemotePath; const MAX_SKILL_DESCRIPTION_CHARS: usize = 512; @@ -18,6 +19,50 @@ lazy_static! { static ref INCOMPLETE_SENTENCE: Regex = Regex::new(r"[^.!?]*$").expect("Incomplete sentence regex should be valid"); } +/// Parse skill markdown content that was fetched outside the local filesystem. +/// +/// This is used for remote project skills, whose SKILL.md body arrives through +/// the remote file-read transport rather than `std::fs`. +pub fn parse_skill_content_at_location( + path: LocalOrRemotePath, + content: &str, + provider: SkillProvider, + scope: SkillScope, +) -> Result { + let parsed = parse_markdown_content(content)?; + let name = match parsed + .front_matter + .get("name") + .map(|value| value.trim()) + .filter(|value| !value.is_empty()) + { + Some(name) => name.to_string(), + None => derive_skill_name_from_path(&path)?, + }; + + let description = match parsed + .front_matter + .get("description") + .map(|value| value.trim()) + .filter(|value| !value.is_empty()) + { + Some(description) => description.to_string(), + None => truncate_skill_description( + &derive_description_from_content(&parsed.content, parsed.line_range.as_ref()) + .unwrap_or_default(), + ), + }; + + Ok(ParsedSkill { + path, + name, + description, + content: parsed.content, + line_range: parsed.line_range, + provider, + scope, + }) +} #[derive(Error, Debug)] pub enum ParseSkillError { @@ -30,7 +75,7 @@ pub enum ParseSkillError { /// Represents a parsed skill with validated fields #[derive(Debug, Clone, PartialEq)] pub struct ParsedSkill { - pub path: PathBuf, + pub path: LocalOrRemotePath, pub name: String, pub description: String, /// The entire content of the file (including front matter) @@ -53,7 +98,7 @@ impl ParsedSkill { impl Display for ParsedSkill { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "Skill: {}", self.path.display()) + write!(f, "Skill: {}", self.path.display_path()) } } @@ -67,7 +112,11 @@ impl Display for ParsedSkill { pub fn parse_skill(path: &Path) -> Result { let provider = get_provider_for_path(path).unwrap_or(SkillProvider::Agents); let scope = get_scope_for_path(path); - parse_skill_internal(path, provider, scope) + parse_skill_internal( + LocalOrRemotePath::Local(path.to_path_buf()), + provider, + scope, + ) } /// Parse a bundled skill markdown file. @@ -82,55 +131,28 @@ pub fn parse_skill(path: &Path) -> Result { /// # Returns /// * `Result` - Parsed skill with validated name and description pub fn parse_bundled_skill(path: &Path) -> Result { - parse_skill_internal(path, SkillProvider::Warp, SkillScope::Bundled) + parse_skill_internal( + LocalOrRemotePath::Local(path.to_path_buf()), + SkillProvider::Warp, + SkillScope::Bundled, + ) } fn parse_skill_internal( - path: &Path, + path: LocalOrRemotePath, provider: SkillProvider, scope: SkillScope, ) -> Result { - let parsed = parse_markdown_file(path)?; - - let name = match parsed - .front_matter - .get("name") - .map(|value| value.trim()) - .filter(|value| !value.is_empty()) - { - Some(name) => name.to_string(), - None => derive_skill_name_from_path(path)?, - }; - - let description = match parsed - .front_matter - .get("description") - .map(|value| value.trim()) - .filter(|value| !value.is_empty()) - { - Some(description) => description.to_string(), - None => truncate_skill_description( - &derive_description_from_content(&parsed.content, parsed.line_range.as_ref()) - .unwrap_or_default(), - ), - }; - - Ok(ParsedSkill { - path: path.to_path_buf(), - name, - description, - content: parsed.content, - line_range: parsed.line_range, - provider, - scope, - }) + let local_path = path + .to_local_path() + .expect("parse_skill_internal only reads local files"); + let content = fs::read_to_string(local_path)?; + parse_skill_content_at_location(path, &content, provider, scope) } -fn derive_skill_name_from_path(path: &Path) -> Result { +fn derive_skill_name_from_path(path: &LocalOrRemotePath) -> Result { path.parent() - .and_then(|parent| parent.file_name()) - .and_then(|name| name.to_str()) - .map(|name| name.to_string()) + .and_then(|parent| parent.file_name().map(str::to_owned)) .ok_or(ParseSkillError::CouldNotDeriveSkillNameFromPath.into()) } diff --git a/crates/ai/src/skills/parse_skill_tests.rs b/crates/ai/src/skills/parse_skill_tests.rs index 104782b0b6..9054c49e9a 100644 --- a/crates/ai/src/skills/parse_skill_tests.rs +++ b/crates/ai/src/skills/parse_skill_tests.rs @@ -1,6 +1,7 @@ use std::path::PathBuf; use tempfile::TempDir; +use warp_util::local_or_remote_path::LocalOrRemotePath; use super::*; @@ -49,7 +50,7 @@ Show concrete examples of using this Skill. // Total of 12 lines, so line_range is 5..13 assert_eq!(result.line_range, Some(5..13)); // Verify path is the full file path - assert_eq!(result.path, skill_file); + assert_eq!(result.path, LocalOrRemotePath::Local(skill_file)); } #[test] diff --git a/crates/ai/src/skills/parser.rs b/crates/ai/src/skills/parser.rs index cdc8a08abc..b41d4de198 100644 --- a/crates/ai/src/skills/parser.rs +++ b/crates/ai/src/skills/parser.rs @@ -1,7 +1,5 @@ use std::collections::HashMap; -use std::fs; use std::ops::Range; -use std::path::Path; use anyhow::{Context, Result}; use regex::Regex; @@ -21,19 +19,6 @@ pub struct ParsedMarkdown { pub line_range: Option>, } -/// Parse a markdown file with YAML front matter -/// -/// # Arguments -/// * `path` - Path to the markdown file to parse -/// -/// # Returns -/// * `Result` - Parsed document with front matter and content -#[allow(dead_code)] -pub fn parse_markdown_file(path: &Path) -> Result { - let content = fs::read_to_string(path)?; - parse_markdown_content(&content) -} - /// Parse markdown content with YAML front matter #[allow(dead_code)] pub(crate) fn parse_markdown_content(content: &str) -> Result { diff --git a/crates/ai/src/skills/read_skills_tests.rs b/crates/ai/src/skills/read_skills_tests.rs index 4af237eaf8..0ad257116c 100644 --- a/crates/ai/src/skills/read_skills_tests.rs +++ b/crates/ai/src/skills/read_skills_tests.rs @@ -1,6 +1,7 @@ use std::fs; use tempfile::tempdir; +use warp_util::local_or_remote_path::LocalOrRemotePath; use super::*; @@ -51,7 +52,7 @@ This is the second test skill. let skill1 = skills.iter().find(|s| s.name == "test-skill-1").unwrap(); assert_eq!( skill1.path, - skill1_dir.join("SKILL.md").to_string_lossy().to_string() + LocalOrRemotePath::Local(skill1_dir.join("SKILL.md")) ); assert_eq!(skill1.description, "First test skill"); assert!(skill1.content.contains("# Test Skill 1")); @@ -62,7 +63,7 @@ This is the second test skill. let skill2 = skills.iter().find(|s| s.name == "test-skill-2").unwrap(); assert_eq!( skill2.path, - skill2_dir.join("SKILL.md").to_string_lossy().to_string() + LocalOrRemotePath::Local(skill2_dir.join("SKILL.md")) ); assert_eq!(skill2.description, "Second test skill"); assert!(skill2.content.contains("# Test Skill 2")); diff --git a/crates/ai/src/skills/skill_reference.rs b/crates/ai/src/skills/skill_reference.rs index 1c437eb1fd..fe9faeacfc 100644 --- a/crates/ai/src/skills/skill_reference.rs +++ b/crates/ai/src/skills/skill_reference.rs @@ -1,13 +1,13 @@ use std::fmt; -use std::path::PathBuf; use serde::{Deserialize, Serialize}; +use warp_util::local_or_remote_path::LocalOrRemotePath; /// An unique reference to a skill. #[derive(Debug, Clone, PartialEq, Eq, Hash, Deserialize, Serialize)] pub enum SkillReference { /// A skill identified by the path to its SKILL.md file. - Path(PathBuf), + Path(LocalOrRemotePath), /// A bundled skill distributed with Warp. BundledSkillId(String), } @@ -15,7 +15,7 @@ pub enum SkillReference { impl fmt::Display for SkillReference { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { - SkillReference::Path(path) => path.display().fmt(f), + SkillReference::Path(path) => path.display_path().fmt(f), SkillReference::BundledSkillId(id) => write!(f, "@warp-skill:{id}"), } } @@ -25,9 +25,7 @@ impl From for warp_multi_agent_api::skill_descriptor::SkillRefer fn from(reference: SkillReference) -> Self { match reference { SkillReference::Path(path) => { - warp_multi_agent_api::skill_descriptor::SkillReference::Path( - path.to_string_lossy().to_string(), - ) + warp_multi_agent_api::skill_descriptor::SkillReference::Path(path.display_path()) } SkillReference::BundledSkillId(id) => { warp_multi_agent_api::skill_descriptor::SkillReference::BundledSkillId(id) diff --git a/crates/warp_util/src/local_or_remote_path.rs b/crates/warp_util/src/local_or_remote_path.rs index 871c9c6d97..1c8e4a1f27 100644 --- a/crates/warp_util/src/local_or_remote_path.rs +++ b/crates/warp_util/src/local_or_remote_path.rs @@ -56,6 +56,41 @@ impl LocalOrRemotePath { } } + /// Returns this location's parent, preserving remote host identity. + pub fn parent(&self) -> Option { + match self { + LocalOrRemotePath::Local(path) => path + .parent() + .map(|parent| LocalOrRemotePath::Local(parent.to_path_buf())), + LocalOrRemotePath::Remote(remote) => remote.path.parent().map(|parent| { + LocalOrRemotePath::Remote(RemotePath::new(remote.host_id.clone(), parent)) + }), + } + } + + /// Returns the file name component, regardless of where the path lives. + pub fn file_name(&self) -> Option<&str> { + match self { + LocalOrRemotePath::Local(path) => path.file_name().and_then(|name| name.to_str()), + LocalOrRemotePath::Remote(remote) => remote.path.file_name(), + } + } + + /// Returns whether this location starts with `base`. + /// + /// Remote locations only compare as ancestors when they are on the same + /// host. This prevents `/repo` on one host from matching `/repo` on another. + pub fn starts_with(&self, base: &LocalOrRemotePath) -> bool { + match (self, base) { + (LocalOrRemotePath::Local(path), LocalOrRemotePath::Local(base)) => { + path.starts_with(base) + } + (LocalOrRemotePath::Remote(path), LocalOrRemotePath::Remote(base)) => { + path.host_id == base.host_id && path.path.starts_with(&base.path) + } + _ => false, + } + } /// Returns the local path if this is a `Local` location, `None` for `Remote`. /// Callers that only work with local files (LSP, save-to-disk, reveal-in-finder) /// should use this to gate their behavior. From f0bdfe55547e8a88cfc319799439acf5bf210584 Mon Sep 17 00:00:00 2001 From: Moira Huang Date: Fri, 29 May 2026 14:53:25 -0700 Subject: [PATCH 2/4] specify local more --- app/src/ai/agent_sdk/driver.rs | 7 +- app/src/ai/skills/global_skills.rs | 14 ++-- app/src/ai/skills/global_skills_tests.rs | 96 +++++++++++++++++++++--- crates/ai/src/skills/parse_skill.rs | 30 +++----- 4 files changed, 111 insertions(+), 36 deletions(-) diff --git a/app/src/ai/agent_sdk/driver.rs b/app/src/ai/agent_sdk/driver.rs index 3cfd258bfe..d30c74dcc1 100644 --- a/app/src/ai/agent_sdk/driver.rs +++ b/app/src/ai/agent_sdk/driver.rs @@ -28,6 +28,7 @@ use warp_core::features::FeatureFlag; use warp_core::{report_error, report_if_error, safe_debug, safe_error, safe_info}; use warp_graphql::ai::AgentTaskState; use warp_managed_secrets::ManagedSecretValue; +use warp_util::local_or_remote_path::LocalOrRemotePath; use warpui::r#async::{FutureExt, TimeoutError}; use warpui::{AppContext, Entity, ModelContext, ModelHandle, ModelSpawner, SingletonEntity}; @@ -1552,7 +1553,11 @@ impl AgentDriver { .iter() .map(|def| repo_path.join(&def.skills_path)); let repo_skills = read_skills_from_directories(skill_dirs); - let filtered = filter_skills_by_spec(&repo_path, repo_skills, &specs); + let filtered = filter_skills_by_spec( + &LocalOrRemotePath::Local(repo_path), + repo_skills, + &specs, + ); all_skills.extend(filtered); } all_skills diff --git a/app/src/ai/skills/global_skills.rs b/app/src/ai/skills/global_skills.rs index 3e580cffa4..cce20bcaa2 100644 --- a/app/src/ai/skills/global_skills.rs +++ b/app/src/ai/skills/global_skills.rs @@ -3,11 +3,10 @@ use std::{ collections::{BTreeSet, HashMap, HashSet}, - path::Path, str::FromStr, }; -use ai::skills::{provider_rank, ParsedSkill}; +use ai::skills::{ParsedSkill, provider_rank}; use warp_cli::skill::SkillSpec; use warp_util::local_or_remote_path::LocalOrRemotePath; @@ -49,7 +48,7 @@ pub fn resolve_skill_repos(raw_specs: &[String]) -> (Vec, Vec, specs: &[SkillSpec], ) -> Vec { @@ -80,27 +79,26 @@ pub fn filter_skills_by_spec( } fn matching_skill_path( - repo_path: &Path, + repo_path: &LocalOrRemotePath, skills_by_path: &HashMap, spec: &SkillSpec, ) -> Option { if spec.is_full_path() { - let path = LocalOrRemotePath::Local(repo_path.join(&spec.skill_identifier)); + let path = repo_path.join(&spec.skill_identifier); return skills_by_path.contains_key(&path).then_some(path); } matching_simple_skill_path(repo_path, skills_by_path, &spec.skill_identifier) } fn matching_simple_skill_path( - repo_path: &Path, + repo_path: &LocalOrRemotePath, skills_by_path: &HashMap, skill_name: &str, ) -> Option { - let repo_path = LocalOrRemotePath::Local(repo_path.to_path_buf()); let mut matches = skills_by_path .values() .copied() - .filter(|skill| skill.path.starts_with(&repo_path) && skill.name == skill_name) + .filter(|skill| skill.path.starts_with(repo_path) && skill.name == skill_name) .collect::>(); matches.sort_by(|left, right| { diff --git a/app/src/ai/skills/global_skills_tests.rs b/app/src/ai/skills/global_skills_tests.rs index 636fd393da..e9a8969ca8 100644 --- a/app/src/ai/skills/global_skills_tests.rs +++ b/app/src/ai/skills/global_skills_tests.rs @@ -1,9 +1,12 @@ use std::path::{Path, PathBuf}; use std::str::FromStr; -use ai::skills::{get_provider_for_path, ParsedSkill, SkillProvider, SkillScope}; +use ai::skills::{ParsedSkill, SkillProvider, SkillScope, get_provider_for_path}; use warp_cli::skill::SkillSpec; +use warp_util::host_id::HostId; use warp_util::local_or_remote_path::LocalOrRemotePath; +use warp_util::remote_path::RemotePath; +use warp_util::standardized_path::StandardizedPath; use super::{filter_skills_by_spec, resolve_skill_repos}; use crate::ai::cloud_environments::GithubRepo; @@ -72,11 +75,58 @@ fn filter_skills_by_spec_only_loads_requested_simple_names() { ]; let specs = global_specs(&["warpdotdev/warp-internal:read-google-doc".to_string()]); - let filtered = filter_skills_by_spec(repo_path, skills, &specs); + let filtered = filter_skills_by_spec( + &LocalOrRemotePath::Local(repo_path.to_path_buf()), + skills, + &specs, + ); assert_eq!(skill_paths(filtered), vec![requested_skill_path]); } +#[test] +fn filter_skills_by_spec_matches_full_path_specs_for_remote_repos() { + let repo_path = remote_path("host-a", "/work/warp-internal"); + let requested_skill_path = repo_path.join(".claude/skills/deploy/SKILL.md"); + let other_host_skill_path = + remote_path("host-b", "/work/warp-internal").join(".claude/skills/deploy/SKILL.md"); + let skills = vec![ + parsed_skill_at_location(other_host_skill_path, "deploy", SkillProvider::Claude), + parsed_skill_at_location( + requested_skill_path.clone(), + "deploy", + SkillProvider::Claude, + ), + ]; + let specs = + global_specs(&["warpdotdev/warp-internal:.claude/skills/deploy/SKILL.md".to_string()]); + + let filtered = filter_skills_by_spec(&repo_path, skills, &specs); + + assert_eq!(skill_locations(filtered), vec![requested_skill_path]); +} + +#[test] +fn filter_skills_by_spec_scopes_simple_remote_names_to_the_repo_host() { + let repo_path = remote_path("host-a", "/work/warp-internal"); + let requested_skill_path = repo_path.join(".claude/skills/deploy/SKILL.md"); + let other_host_skill_path = + remote_path("host-b", "/work/warp-internal").join(".agents/skills/deploy/SKILL.md"); + let skills = vec![ + parsed_skill_at_location(other_host_skill_path, "deploy", SkillProvider::Agents), + parsed_skill_at_location( + requested_skill_path.clone(), + "deploy", + SkillProvider::Claude, + ), + ]; + let specs = global_specs(&["warpdotdev/warp-internal:deploy".to_string()]); + + let filtered = filter_skills_by_spec(&repo_path, skills, &specs); + + assert_eq!(skill_locations(filtered), vec![requested_skill_path]); +} + #[test] fn filter_skills_by_spec_matches_simple_names_by_parsed_skill_name() { let repo_path = Path::new("/work/warp-internal"); @@ -87,8 +137,11 @@ fn filter_skills_by_spec_matches_simple_names_by_parsed_skill_name() { parsed_skill(directory_name_match_path, "unrelated-skill"), ]; let specs = global_specs(&["warpdotdev/warp-internal:read-google-doc".to_string()]); - - let filtered = filter_skills_by_spec(repo_path, skills, &specs); + let filtered = filter_skills_by_spec( + &LocalOrRemotePath::Local(repo_path.to_path_buf()), + skills, + &specs, + ); assert_eq!(skill_paths(filtered), vec![requested_skill_path]); } @@ -103,8 +156,11 @@ fn filter_skills_by_spec_uses_provider_precedence_for_simple_names() { parsed_skill(agents_skill_path.clone(), "deploy"), ]; let specs = global_specs(&["warpdotdev/warp-internal:deploy".to_string()]); - - let filtered = filter_skills_by_spec(repo_path, skills, &specs); + let filtered = filter_skills_by_spec( + &LocalOrRemotePath::Local(repo_path.to_path_buf()), + skills, + &specs, + ); assert_eq!(skill_paths(filtered), vec![agents_skill_path]); } @@ -126,8 +182,11 @@ fn filter_skills_by_spec_matches_full_path_specs() { "warpdotdev/warp-internal:{}", requested_relative_path.display() )]); - - let filtered = filter_skills_by_spec(repo_path, skills, &specs); + let filtered = filter_skills_by_spec( + &LocalOrRemotePath::Local(repo_path.to_path_buf()), + skills, + &specs, + ); assert_eq!(skill_paths(filtered), vec![requested_skill_path]); } @@ -149,8 +208,16 @@ fn skill_path(repo_path: &Path, provider_dir: &str, skill_name: &str) -> PathBuf fn parsed_skill(path: PathBuf, name: &str) -> ParsedSkill { let provider = get_provider_for_path(&path).unwrap_or(SkillProvider::Agents); + parsed_skill_at_location(LocalOrRemotePath::Local(path), name, provider) +} + +fn parsed_skill_at_location( + path: LocalOrRemotePath, + name: &str, + provider: SkillProvider, +) -> ParsedSkill { ParsedSkill { - path: LocalOrRemotePath::Local(path), + path, name: name.to_string(), description: String::new(), content: String::new(), @@ -167,6 +234,17 @@ fn skill_paths(skills: Vec) -> Vec { .collect() } +fn skill_locations(skills: Vec) -> Vec { + skills.into_iter().map(|skill| skill.path).collect() +} + +fn remote_path(host_id: &str, path: &str) -> LocalOrRemotePath { + LocalOrRemotePath::Remote(RemotePath::new( + HostId::new(host_id.to_string()), + StandardizedPath::try_new(path).unwrap(), + )) +} + #[test] fn resolve_skill_repos_collapses_duplicates_preserving_first_seen_order() { let (_specs, repos) = resolve_skill_repos(&[ diff --git a/crates/ai/src/skills/parse_skill.rs b/crates/ai/src/skills/parse_skill.rs index dd5c3a9aa7..cecf8705b9 100644 --- a/crates/ai/src/skills/parse_skill.rs +++ b/crates/ai/src/skills/parse_skill.rs @@ -4,7 +4,7 @@ use std::ops::Range; use std::path::Path; use super::parser::parse_markdown_content; -use super::skill_provider::{get_provider_for_path, get_scope_for_path, SkillProvider, SkillScope}; +use super::skill_provider::{SkillProvider, SkillScope, get_provider_for_path, get_scope_for_path}; use anyhow::Result; use lazy_static::lazy_static; use regex::Regex; @@ -112,11 +112,7 @@ impl Display for ParsedSkill { pub fn parse_skill(path: &Path) -> Result { let provider = get_provider_for_path(path).unwrap_or(SkillProvider::Agents); let scope = get_scope_for_path(path); - parse_skill_internal( - LocalOrRemotePath::Local(path.to_path_buf()), - provider, - scope, - ) + parse_local_skill_internal(path, provider, scope) } /// Parse a bundled skill markdown file. @@ -131,23 +127,21 @@ pub fn parse_skill(path: &Path) -> Result { /// # Returns /// * `Result` - Parsed skill with validated name and description pub fn parse_bundled_skill(path: &Path) -> Result { - parse_skill_internal( - LocalOrRemotePath::Local(path.to_path_buf()), - SkillProvider::Warp, - SkillScope::Bundled, - ) + parse_local_skill_internal(path, SkillProvider::Warp, SkillScope::Bundled) } -fn parse_skill_internal( - path: LocalOrRemotePath, +fn parse_local_skill_internal( + path: &Path, provider: SkillProvider, scope: SkillScope, ) -> Result { - let local_path = path - .to_local_path() - .expect("parse_skill_internal only reads local files"); - let content = fs::read_to_string(local_path)?; - parse_skill_content_at_location(path, &content, provider, scope) + let content = fs::read_to_string(path)?; + parse_skill_content_at_location( + LocalOrRemotePath::Local(path.to_path_buf()), + &content, + provider, + scope, + ) } fn derive_skill_name_from_path(path: &LocalOrRemotePath) -> Result { From a901b305888454c64f7525e2c752b47b57545540 Mon Sep 17 00:00:00 2001 From: Moira Huang Date: Fri, 29 May 2026 15:19:43 -0700 Subject: [PATCH 3/4] fix wasm --- app/src/ai/skills/dummy_skill_manager.rs | 15 ++++++++++----- app/src/ai/skills/mod.rs | 24 ++++++++++++++++++++++++ app/src/ai/skills/skill_manager.rs | 23 +---------------------- 3 files changed, 35 insertions(+), 27 deletions(-) diff --git a/app/src/ai/skills/dummy_skill_manager.rs b/app/src/ai/skills/dummy_skill_manager.rs index cdff7ef428..056973735b 100644 --- a/app/src/ai/skills/dummy_skill_manager.rs +++ b/app/src/ai/skills/dummy_skill_manager.rs @@ -2,7 +2,7 @@ use ai::skills::{ParsedSkill, SkillProvider, SkillReference}; use warp_util::local_or_remote_path::LocalOrRemotePath; use warpui::{AppContext, Entity, ModelContext, SingletonEntity}; -use crate::ai::skills::SkillDescriptor; +use crate::ai::skills::{SkillDescriptor, SkillPathQuery}; pub struct SkillManager {} @@ -19,12 +19,17 @@ impl SkillManager { vec![] } - pub fn skill_by_path(&self, _skill_path: &LocalOrRemotePath) -> Option<&ParsedSkill> { + pub fn skill_by_path( + &self, + _skill_path: &P, + ) -> Option<&ParsedSkill> { None } - - pub fn reference_for_skill_path(&self, skill_path: &LocalOrRemotePath) -> SkillReference { - SkillReference::Path(skill_path.clone()) + pub fn reference_for_skill_path( + &self, + skill_path: &P, + ) -> SkillReference { + SkillReference::Path(skill_path.to_skill_location()) } pub fn skill_by_reference(&self, _reference: &SkillReference) -> Option<&ParsedSkill> { diff --git a/app/src/ai/skills/mod.rs b/app/src/ai/skills/mod.rs index 745954763a..ee6c4edfcb 100644 --- a/app/src/ai/skills/mod.rs +++ b/app/src/ai/skills/mod.rs @@ -1,3 +1,6 @@ +use std::path::{Path, PathBuf}; + +use warp_util::local_or_remote_path::LocalOrRemotePath; mod telemetry; pub use telemetry::{SkillOpenOrigin, SkillTelemetryEvent}; @@ -23,6 +26,27 @@ pub use skill_utils::{ icon_override_for_skill_name, list_skills_if_changed, render_skill_button, skill_path_from_file_path, }; +pub trait SkillPathQuery { + fn to_skill_location(&self) -> LocalOrRemotePath; +} + +impl SkillPathQuery for LocalOrRemotePath { + fn to_skill_location(&self) -> LocalOrRemotePath { + self.clone() + } +} + +impl SkillPathQuery for Path { + fn to_skill_location(&self) -> LocalOrRemotePath { + LocalOrRemotePath::Local(self.to_path_buf()) + } +} + +impl SkillPathQuery for PathBuf { + fn to_skill_location(&self) -> LocalOrRemotePath { + LocalOrRemotePath::Local(self.clone()) + } +} #[cfg(not(target_family = "wasm"))] mod resolve_skill_spec; diff --git a/app/src/ai/skills/skill_manager.rs b/app/src/ai/skills/skill_manager.rs index a4275c3b89..cba84243b8 100644 --- a/app/src/ai/skills/skill_manager.rs +++ b/app/src/ai/skills/skill_manager.rs @@ -14,7 +14,7 @@ use warp_core::{report_error, safe_warn}; use warp_util::local_or_remote_path::LocalOrRemotePath; use warpui::{AppContext, Entity, ModelContext, ModelHandle, SingletonEntity}; -use super::SkillDescriptor; +use super::{SkillDescriptor, SkillPathQuery}; use crate::ai::mcp::{McpIntegration, TemplatableMCPServerManager}; use crate::ai::skills::skill_utils::unique_skills; use crate::keyboard::keybinding_file_path; @@ -79,27 +79,6 @@ pub struct SkillManager { skill_watcher: ModelHandle, // Can't remove this or it'll get cleaned up after new() } -pub trait SkillPathQuery { - fn to_skill_location(&self) -> LocalOrRemotePath; -} - -impl SkillPathQuery for LocalOrRemotePath { - fn to_skill_location(&self) -> LocalOrRemotePath { - self.clone() - } -} - -impl SkillPathQuery for Path { - fn to_skill_location(&self) -> LocalOrRemotePath { - LocalOrRemotePath::Local(self.to_path_buf()) - } -} - -impl SkillPathQuery for PathBuf { - fn to_skill_location(&self) -> LocalOrRemotePath { - LocalOrRemotePath::Local(self.clone()) - } -} impl SkillManager { pub fn new(ctx: &mut ModelContext) -> Self { let (skill_watcher_tx, skill_watcher_rx) = async_channel::unbounded(); From 0420b8cbc011c9f2ce68658024c7d597d92bd94d Mon Sep 17 00:00:00 2001 From: Moira Huang Date: Fri, 29 May 2026 15:45:05 -0700 Subject: [PATCH 4/4] fix windows --- app/src/ai/skills/file_watchers/utils.rs | 30 ++++++++++++++---------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/app/src/ai/skills/file_watchers/utils.rs b/app/src/ai/skills/file_watchers/utils.rs index 0107a9b572..3f0e02384b 100644 --- a/app/src/ai/skills/file_watchers/utils.rs +++ b/app/src/ai/skills/file_watchers/utils.rs @@ -227,20 +227,26 @@ pub fn extract_skill_parent_directory( }; for definition in SKILL_PROVIDER_DEFINITIONS.iter() { - if !skills_root - .path_component() - .ends_with(&definition.skills_path.to_string_lossy()) - { - continue; - } - let mut parent_directory = skills_root.clone(); - for _ in definition.skills_path.components() { - parent_directory = parent_directory - .parent() - .ok_or_else(|| anyhow::anyhow!("Not a skill path: {}", path.display_path()))?; + let mut matches_provider = true; + for component in definition.skills_path.components().rev() { + let Some(expected_component) = component.as_os_str().to_str() else { + matches_provider = false; + break; + }; + if parent_directory.file_name() != Some(expected_component) { + matches_provider = false; + break; + } + let Some(parent) = parent_directory.parent() else { + matches_provider = false; + break; + }; + parent_directory = parent; + } + if matches_provider { + return Ok(parent_directory); } - return Ok(parent_directory); } Err(anyhow::anyhow!("Not a skill path: {}", path.display_path()))