Skip to content

Commit 729e0a8

Browse files
committed
fix(summon): return descriptive errors for unreadable supporting files
When a matched supporting file can't be read (permissions, non-UTF8), resolve_source now returns an error describing the actual failure instead of returning None which surfaced as a misleading "not found" message. Changed the return type to Result<Option<Source>, String> to distinguish "not found" from "found but failed to read".
1 parent ec87472 commit 729e0a8

File tree

1 file changed

+20
-30
lines changed
  • crates/goose/src/agents/platform_extensions

1 file changed

+20
-30
lines changed

crates/goose/src/agents/platform_extensions/summon.rs

Lines changed: 20 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -693,14 +693,14 @@ impl SummonClient {
693693
session_id: &str,
694694
name: &str,
695695
working_dir: &Path,
696-
) -> Option<Source> {
696+
) -> Result<Option<Source>, String> {
697697
let sources = self.get_sources(session_id, working_dir).await;
698698

699699
if let Some(mut source) = sources.iter().find(|s| s.name == name).cloned() {
700700
if source.kind == SourceKind::Subrecipe && source.content.is_empty() {
701701
source.content = self.load_subrecipe_content(session_id, &source.name).await;
702702
}
703-
return Some(source);
703+
return Ok(Some(source));
704704
}
705705

706706
if let Some((skill_name, relative_path)) = name.split_once('/') {
@@ -722,39 +722,29 @@ impl SummonClient {
722722
.map(|p| p.starts_with(&canonical_skill_dir))
723723
.unwrap_or(false);
724724
if !safe {
725-
warn!(
726-
"Refusing to load supporting file outside skill directory: {}",
727-
file_path.display()
728-
);
729-
return None;
730-
}
731-
match std::fs::read_to_string(file_path) {
732-
Ok(content) => {
733-
return Some(Source {
734-
name: name.to_string(),
735-
kind: SourceKind::Skill,
736-
description: format!("Supporting file for {}", skill_name),
737-
path: file_path.clone(),
738-
content,
739-
supporting_files: vec![],
740-
});
741-
}
742-
Err(e) => {
743-
warn!(
744-
"Failed to read supporting file {}: {}",
745-
file_path.display(),
746-
e
747-
);
748-
return None;
749-
}
725+
return Err(format!(
726+
"Refusing to load '{}': file resolves outside the skill directory",
727+
name
728+
));
750729
}
730+
return match std::fs::read_to_string(file_path) {
731+
Ok(content) => Ok(Some(Source {
732+
name: name.to_string(),
733+
kind: SourceKind::Skill,
734+
description: format!("Supporting file for {}", skill_name),
735+
path: file_path.clone(),
736+
content,
737+
supporting_files: vec![],
738+
})),
739+
Err(e) => Err(format!("Failed to read '{}': {}", name, e)),
740+
};
751741
}
752742
}
753743
}
754744
}
755745
}
756746

757-
None
747+
Ok(None)
758748
}
759749

760750
async fn load_subrecipe_content(&self, session_id: &str, name: &str) -> String {
@@ -1094,7 +1084,7 @@ impl SummonClient {
10941084
name: &str,
10951085
working_dir: &Path,
10961086
) -> Result<Vec<Content>, String> {
1097-
let source = self.resolve_source(session_id, name, working_dir).await;
1087+
let source = self.resolve_source(session_id, name, working_dir).await?;
10981088

10991089
match source {
11001090
Some(source) => {
@@ -1326,7 +1316,7 @@ impl SummonClient {
13261316
) -> Result<Recipe, String> {
13271317
let source = self
13281318
.resolve_source(session_id, source_name, working_dir)
1329-
.await
1319+
.await?
13301320
.ok_or_else(|| format!("Source '{}' not found", source_name))?;
13311321

13321322
let mut recipe = match source.kind {

0 commit comments

Comments
 (0)