Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/goose-cli/src/scenario_tests/mock_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ impl McpClientTrait for MockClient {
}

fn get_info(&self) -> std::option::Option<&rmcp::model::InitializeResult> {
todo!()
None
}

async fn read_resource(
Expand Down
4 changes: 1 addition & 3 deletions crates/goose/src/agents/extension_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,7 @@ impl Extension {
}

fn get_instructions(&self) -> Option<String> {
self.server_info
.as_ref()
.and_then(|info| info.instructions.clone())
self.client.get_instructions()
Comment on lines 108 to +109

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid rescanning skills synchronously on each prompt turn

Delegating Extension::get_instructions() to client.get_instructions() now makes the skills extension run discover_skills() on every get_extensions_info() call (which happens whenever the system prompt is rebuilt each turn). discover_skills() performs recursive std::fs walking, so this introduces blocking filesystem I/O in the hot path and can noticeably increase response latency when users have many skills/supporting files. Consider caching/invalidation or moving the scan off the async worker path.

Useful? React with 👍 / 👎.

}

fn get_client(&self) -> McpClientBox {
Expand Down
7 changes: 7 additions & 0 deletions crates/goose/src/agents/mcp_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,13 @@ pub trait McpClientTrait: Send + Sync {

fn get_info(&self) -> Option<&InitializeResult>;

/// Return the extension's current instructions. The default reads from
/// `get_info()`, but platform extensions can override this to provide
/// dynamically computed instructions (e.g. freshly discovered skills).
fn get_instructions(&self) -> Option<String> {
self.get_info().and_then(|info| info.instructions.clone())
}

async fn list_resources(
&self,
_session_id: &str,
Expand Down
8 changes: 3 additions & 5 deletions crates/goose/src/agents/prompt_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -441,11 +441,9 @@ mod tests {
.values()
.map(|def| {
let client = (def.client_factory)(context.clone());
let info = client.get_info();
let instructions = info
.and_then(|i| i.instructions.clone())
.unwrap_or_default();
let has_resources = info
let instructions = client.get_instructions().unwrap_or_default();
let has_resources = client
.get_info()
.and_then(|i| i.capabilities.resources.as_ref())
.is_some();
ExtensionInfo::new(def.name, &instructions, has_resources)
Expand Down
47 changes: 24 additions & 23 deletions crates/goose/src/skills/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,30 +27,8 @@ impl SkillsClient {
.map(|s| s.working_dir.clone())
.unwrap_or_else(|| std::env::current_dir().unwrap_or_default());

let mut instructions = String::new();
if context.session.is_some() {
let sources = discover_skills(Some(&working_dir));
let mut skills: Vec<&SourceEntry> = sources
.iter()
.filter(|s| {
s.source_type == SourceType::Skill || s.source_type == SourceType::BuiltinSkill
})
.collect();
skills.sort_by(|a, b| (&a.name, &a.path).cmp(&(&b.name, &b.path)));

if !skills.is_empty() {
instructions.push_str(
"\n\nYou have these skills at your disposal, when it is clear they can help you solve a problem or you are asked to use them:",
);
for skill in &skills {
instructions.push_str(&format!("\n• {} - {}", skill.name, skill.description));
}
}
}

let info = InitializeResult::new(ServerCapabilities::builder().enable_tools().build())
.with_server_info(Implementation::new(EXTENSION_NAME, "1.0.0").with_title("Skills"))
.with_instructions(instructions);
.with_server_info(Implementation::new(EXTENSION_NAME, "1.0.0").with_title("Skills"));

Ok(Self { info, working_dir })
}
Expand Down Expand Up @@ -252,6 +230,29 @@ impl McpClientTrait for SkillsClient {
Some(&self.info)
}

fn get_instructions(&self) -> Option<String> {
let sources = discover_skills(Some(&self.working_dir));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Skip skill discovery when session context is missing

get_instructions() now always scans self.working_dir, but SkillsClient::new() falls back to std::env::current_dir() when no session is attached. In production, extensions can be added with session_id: None (for example via the extension-manager tool path), so this change can inject skills from the process CWD instead of the active session/project directory into the system prompt. Before this commit, instructions were not emitted in the no-session case, so this is a behavioral regression that can surface incorrect or unrelated skills.

Useful? React with 👍 / 👎.

let mut skills: Vec<&SourceEntry> = sources
.iter()
.filter(|s| {
s.source_type == SourceType::Skill || s.source_type == SourceType::BuiltinSkill
})
.collect();
skills.sort_by(|a, b| (&a.name, &a.path).cmp(&(&b.name, &b.path)));

if skills.is_empty() {
return None;
}

let mut instructions = String::from(
"\n\nYou have these skills at your disposal, when it is clear they can help you solve a problem or you are asked to use them:",
);
for skill in &skills {
instructions.push_str(&format!("\n• {} - {}", skill.name, skill.description));
}
Some(instructions)
}

async fn subscribe(&self) -> mpsc::Receiver<ServerNotification> {
let (_tx, rx) = mpsc::channel(1);
rx
Expand Down
Loading