-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Support platform tools through CLI #5570
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,7 +28,7 @@ use goose::utils::safe_truncate; | |
|
|
||
| use anyhow::{Context, Result}; | ||
| use completion::GooseCompleter; | ||
| use goose::agents::extension::{Envs, ExtensionConfig}; | ||
| use goose::agents::extension::{Envs, ExtensionConfig, PLATFORM_EXTENSIONS}; | ||
| use goose::agents::types::RetryConfig; | ||
| use goose::agents::{Agent, SessionConfig, MANUAL_COMPACT_TRIGGER}; | ||
| use goose::config::{Config, GooseMode}; | ||
|
|
@@ -299,15 +299,24 @@ impl CliSession { | |
| /// * `builtin_name` - Name of the builtin extension(s), comma separated | ||
| pub async fn add_builtin(&mut self, builtin_name: String) -> Result<()> { | ||
| for name in builtin_name.split(',') { | ||
| let extension_name = name.trim().to_string(); | ||
| let config = ExtensionConfig::Builtin { | ||
| name: extension_name, | ||
| display_name: None, | ||
| // TODO: should set a timeout | ||
| timeout: Some(goose::config::DEFAULT_EXTENSION_TIMEOUT), | ||
| bundled: None, | ||
| description: name.trim().to_string(), | ||
| available_tools: Vec::new(), | ||
| let extension_name = name.trim(); | ||
|
|
||
| let config = if PLATFORM_EXTENSIONS.contains_key(extension_name) { | ||
| ExtensionConfig::Platform { | ||
| name: extension_name.to_string(), | ||
| bundled: None, | ||
| description: name.to_string(), | ||
| available_tools: Vec::new(), | ||
| } | ||
| } else { | ||
| ExtensionConfig::Builtin { | ||
| name: extension_name.to_string(), | ||
| display_name: None, | ||
| timeout: None, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what is the effect of the switch to have no timeout vs the prior ?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interested to know, but doesn't seem like a big deal to me as we wouldn't expect the builtins to time out conceptually |
||
| bundled: None, | ||
| description: name.to_string(), | ||
|
||
| available_tools: Vec::new(), | ||
| } | ||
| }; | ||
| self.agent | ||
| .add_extension(config) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent description field usage. Line 308 uses
name.to_string()while line 317 also usesname.to_string(), butextension_namewas already trimmed on line 302. Line 308 should useextension_name.to_string()to avoid potential whitespace in the description.