filter local skills from remote#12262
Conversation
|
|
||
| if self.is_cloud_environment { | ||
| // In cloud environments, all skills are in scope regardless of cwd. | ||
| // In cloud environments, all skills allowed by the path scope are in scope |
There was a problem hiding this comment.
calling out that i'm setting this as the cloud behavior because i figured that we're intentionally scoping all the paths in the cloud env. i checked this PR for context on expected behavior https://github.com/warpdotdev/warp-internal/commit/d5b60683b14c635c9109bca0cbfa8727467ab151
| if repo_root.as_ref().is_none_or(|root| dir.starts_with(root)) { | ||
| for path in dir_skill_paths { | ||
| skill_paths.push((dir.clone(), path.clone())); | ||
| if path_scope.includes(path) { |
There was a problem hiding this comment.
filter for correct skills
| ), | ||
| ) | ||
| }) | ||
| .unwrap_or((None, SkillPathScope::Local)); |
There was a problem hiding this comment.
figured falling back to local was reasonable
| // regardless of cwd. | ||
| for (dir, dir_skill_paths) in &self.directory_skills { | ||
| if is_home_directory(dir) { | ||
| if is_home_directory(dir) || !path_scope.includes(dir) { |
There was a problem hiding this comment.
this is basically a no-op but i'm adding it to be clear that we should only include "allowed" scopes in the cloud env case as well
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR adds SkillPathScope and threads it through skill listing so local path-based skills are filtered out for remote sessions while bundled and host-matched remote skills remain available.
Concerns
- This change affects user-visible skill lists and slash-command/menu behavior, but the PR description does not include screenshots or a screen recording demonstrating the local-vs-remote filtering end to end. For this user-facing change, please include screenshots or a screen recording demonstrating it working end to end.
Verdict
Found: 0 critical, 0 important, 1 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| pub fn get_skills_for_working_directory( | ||
| &self, | ||
| _working_directory: Option<&LocalOrRemotePath>, | ||
| _path_scope: SkillPathScope, |
There was a problem hiding this comment.
Do we need an additional SkillPathScope here? Could we derive it from working_directory (which has LocalOrRemotePath)
2 participants
Description
We were exposing local skills to remote. This adds a filter to ensure we don't do that
Linked Issue
N/A
ready-to-specorready-to-implement.Testing
This is now WAI because
resolve-merge-conflicts hasn't been installed for my remote yet./script/runScreenshots / Videos
Agent Mode