Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support env vars in systems #574

Open
wants to merge 1 commit into
base: v1.0
Choose a base branch
from
Open
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
11 changes: 0 additions & 11 deletions crates/goose-cli/src/commands/configure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,20 +103,9 @@ pub async fn handle_configure(
.interact()?
};

// Forward any existing systems from the profile if present
Copy link
Collaborator

Choose a reason for hiding this comment

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

I assume this was the unused code?

let additional_systems =
existing_profile.map_or(Vec::new(), |profile| profile.additional_systems.clone());

if !additional_systems.is_empty() {
let _ = cliclack::log::info(
format!("We kept the existing systems from your {} profile. You can edit this with `goose system`", profile_name)
);
}

let profile = Profile {
provider: provider_name.to_string(),
model: model.clone(),
additional_systems,
temperature: None,
context_limit: None,
max_tokens: None,
Expand Down
43 changes: 0 additions & 43 deletions crates/goose-cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ mod logging;
mod profile;
mod prompt;
mod session;
mod systems;

use commands::agent_version::AgentCommand;
use commands::configure::handle_configure;
Expand All @@ -22,8 +21,6 @@ use std::io::{self, Read};
#[cfg(test)]
mod test_helpers;

use crate::systems::system_handler::{add_system, remove_system};

#[derive(Parser)]
#[command(author, about, long_about = None)]
struct Cli {
Expand Down Expand Up @@ -67,13 +64,6 @@ enum Command {
model: Option<String>,
},

/// Manage system prompts and behaviors
#[command(about = "Manage the systems that goose can operate")]
System {
#[command(subcommand)]
action: SystemCommands,
},

/// Manage system prompts and behaviors
#[command(about = "Run one of the mcp servers bundled with goose")]
Mcp { name: String },
Expand Down Expand Up @@ -187,29 +177,6 @@ enum Command {
Agents(AgentCommand),
}

#[derive(Subcommand)]
enum SystemCommands {
/// Add a new system prompt
#[command(about = "Add a new system prompt from URL")]
Add {
#[arg(
help = "URL of the system prompt to add",
long_help = "URL pointing to a file containing the system prompt to be added."
)]
url: String,
},

/// Remove an existing system prompt
#[command(about = "Remove an existing system prompt")]
Remove {
#[arg(
help = "URL of the system prompt to remove",
long_help = "URL of the system prompt that should be removed from the configuration."
)]
url: String,
},
}

#[derive(clap::ValueEnum, Clone, Debug)]
enum CliProviderVariant {
OpenAi,
Expand All @@ -235,16 +202,6 @@ async fn main() -> Result<()> {
let _ = handle_configure(profile_name, provider, model).await;
return Ok(());
}
Some(Command::System { action }) => match action {
SystemCommands::Add { url } => {
add_system(url).await.unwrap();
return Ok(());
}
SystemCommands::Remove { url } => {
remove_system(url).await.unwrap();
return Ok(());
}
},
Some(Command::Mcp { name }) => {
let _ = run_server(&name).await;
}
Expand Down
8 changes: 0 additions & 8 deletions crates/goose-cli/src/profile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ use std::path::PathBuf;
pub struct Profile {
pub provider: String,
pub model: String,
#[serde(default)]
pub additional_systems: Vec<AdditionalSystem>,
pub temperature: Option<f32>,
pub context_limit: Option<usize>,
pub max_tokens: Option<i32>,
Expand All @@ -27,12 +25,6 @@ pub struct Profiles {
pub profile_items: HashMap<String, Profile>,
}

#[derive(Serialize, Deserialize, Clone, Debug)]
pub struct AdditionalSystem {
pub name: String,
pub location: String,
}

pub fn profile_path() -> Result<PathBuf> {
let home_dir = dirs::home_dir().ok_or(anyhow::anyhow!("Could not determine home directory"))?;
let config_dir = home_dir.join(".config").join("goose");
Expand Down
1 change: 0 additions & 1 deletion crates/goose-cli/src/systems/mod.rs

This file was deleted.

93 changes: 0 additions & 93 deletions crates/goose-cli/src/systems/system_handler.rs

This file was deleted.

4 changes: 2 additions & 2 deletions crates/goose/src/agents/capabilities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,8 @@ impl Capabilities {
let transport = SseTransport::new(uri);
McpClient::new(transport.start().await?)
}
SystemConfig::Stdio { ref cmd, ref args } => {
let transport = StdioTransport::new(cmd, args.to_vec());
SystemConfig::Stdio { ref cmd, ref args, ref env } => {
let transport = StdioTransport::new(cmd, args.to_vec()).with_env(env.clone());
McpClient::new(transport.start().await?)
}
};
Expand Down
34 changes: 31 additions & 3 deletions crates/goose/src/agents/system.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use mcp_client::client::Error as ClientError;
use serde::{Deserialize, Serialize};
use std::collections::HashMap;
use thiserror::Error;

/// Errors from System operation
Expand All @@ -22,7 +23,11 @@ pub enum SystemConfig {
/// Server-sent events client with a URI endpoint
Sse { uri: String },
/// Standard I/O client with command and arguments
Stdio { cmd: String, args: Vec<String> },
Stdio {
cmd: String,
args: Vec<String>,
env: Option<HashMap<String, String>>,
},
}

impl SystemConfig {
Expand All @@ -34,6 +39,7 @@ impl SystemConfig {
Self::Stdio {
cmd: cmd.into(),
args: vec![],
env: None,
}
}

Expand All @@ -43,9 +49,26 @@ impl SystemConfig {
S: Into<String>,
{
match self {
Self::Stdio { cmd, .. } => Self::Stdio {
Self::Stdio { cmd, env, .. } => Self::Stdio {
cmd,
args: args.into_iter().map(Into::into).collect(),
env,
},
other => other,
}
}

pub fn with_env<I, K, V>(self, env_vars: I) -> Self
where
I: IntoIterator<Item = (K, V)>,
K: Into<String>,
V: Into<String>,
{
match self {
Self::Stdio { cmd, args, .. } => Self::Stdio {
cmd,
args,
env: Some(env_vars.into_iter().map(|(k, v)| (k.into(), v.into())).collect()),
},
other => other,
}
Expand All @@ -56,7 +79,12 @@ impl std::fmt::Display for SystemConfig {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
SystemConfig::Sse { uri } => write!(f, "SSE({})", uri),
SystemConfig::Stdio { cmd, args } => write!(f, "Stdio({} {})", cmd, args.join(" ")),
SystemConfig::Stdio { cmd, args, env } => {
let env_str = env.as_ref().map_or(String::new(), |e| {
format!(" with env: {}", e.iter().map(|(k,v)| format!("{}={}", k, v)).collect::<Vec<_>>().join(","))
});
write!(f, "Stdio({} {}{})", cmd, args.join(" "), env_str)
},
}
}
}
Expand Down
19 changes: 17 additions & 2 deletions crates/mcp-client/src/transport/stdio.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::collections::HashMap;
use std::sync::Arc;
use tokio::process::{Child, ChildStdin, ChildStdout, Command};

Expand Down Expand Up @@ -103,25 +104,39 @@ impl StdioActor {
pub struct StdioTransport {
command: String,
args: Vec<String>,
env: Option<HashMap<String, String>>,
}

impl StdioTransport {
pub fn new<S: Into<String>>(command: S, args: Vec<String>) -> Self {
Self {
command: command.into(),
args,
env: None,
}
}

pub fn with_env(mut self, env: Option<HashMap<String, String>>) -> Self {
self.env = env;
self
}

async fn spawn_process(&self) -> Result<(Child, ChildStdin, ChildStdout), Error> {
let mut process = Command::new(&self.command)
let mut command = Command::new(&self.command);
command
.args(&self.args)
.stdin(std::process::Stdio::piped())
.stdout(std::process::Stdio::piped())
.stderr(std::process::Stdio::inherit())
.kill_on_drop(true)
// 0 sets the process group ID equal to the process ID
.process_group(0) // don't inherit signal handling from parent process
.process_group(0); // don't inherit signal handling from parent process

if let Some(env) = &self.env {
command.envs(env);
}

let mut process = command
.spawn()
.map_err(|e| Error::Other(e.to_string()))?;

Expand Down
Loading