feat: platform extension migrator + code mode rename#6611
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces versioning and migration support for platform extensions, enabling future updates to extension configurations. The primary changes include renaming "Code Execution" to "Code Mode" with an updated description, and adding display_name and version fields to platform extensions.
Changes:
- Added
display_nameandversionfields to platform extension configuration - Implemented migration logic that automatically updates platform extensions when version increases
- Renamed "Code Execution" extension to "Code Mode" with improved description
- Updated TypeScript types and OpenAPI schema to reflect new fields
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| crates/goose/src/agents/extension.rs | Added display_name and version fields to PlatformExtensionDef and ExtensionConfig::Platform; updated all platform extensions with display names and versions |
| crates/goose/src/agents/code_execution_extension.rs | Updated server title from "Code Execution" to "Code Mode" |
| crates/goose/src/config/extensions.rs | Implemented migration logic that compares versions and updates platform extensions while preserving user's enabled state |
| crates/goose/src/recipe/recipe_extension_adapter.rs | Added display_name and version fields to recipe deserialization for platform extensions |
| crates/goose/tests/agent.rs | Updated test fixtures to include new display_name and version fields |
| crates/goose-cli/src/session/mod.rs | Added new fields when creating platform extension configs in CLI |
| crates/goose-acp/src/server.rs | Added new fields when creating platform extension configs in ACP server |
| ui/desktop/openapi.json | Updated OpenAPI schema to include optional display_name and version for platform extensions |
| ui/desktop/src/api/types.gen.ts | Generated TypeScript types reflecting schema changes |
| ui/desktop/src/components/settings/extensions/subcomponents/ExtensionList.tsx | Updated getFriendlyTitle to use display_name for both builtin and platform extensions |
| ui/desktop/src/components/schedule/ScheduleModal.tsx | Added 'platform' to extension type union and updated display_name handling |
| } | ||
|
|
||
| if needs_save { | ||
| save_extensions_map(extensions_map.clone()); |
There was a problem hiding this comment.
The clone operation is unnecessary here. Since extensions_map is returned at the end of the function, you could pass it by value to save_extensions_map instead of cloning it. Consider changing save_extensions_map to take ownership and return the map, or restructure the logic to avoid the clone.
jamadeo
left a comment
There was a problem hiding this comment.
This makes sense to me. But what if we instead just had a "config migrator" that did anything necessary when it sees the config version doesn't match?
It could run any time we load the config file, and migrations could manipulate the document however necessary, including something like this
|
@jamadeo @michaelneale That's a good idea. A versionless generate-diff-and-maybe-update seems better than putting version fields in if it can work. I will attempt tomorrow morning and update this PR. Normally I would just merge and try to improve but this is going to write version values to config files, so I will try to improve it in this way before merge. |
26d851b to
40ccbee
Compare
|
@jamadeo updated to a versionless approach. good call! |
40ccbee to
0e994f7
Compare
| name: def.name.to_string(), | ||
| description: def.description.to_string(), | ||
| display_name: Some(def.display_name.to_string()), | ||
| bundled: Some(true), | ||
| available_tools: Vec::new(), |
There was a problem hiding this comment.
This migration overwrites any existing bundled/available_tools settings for platform extensions (resetting to Some(true) and an empty tool list), which can unintentionally broaden tool availability and discard user config; preserve the existing values when present and only update the fields you intend to migrate (e.g., description/display_name).
| pub struct PlatformExtensionDef { | ||
| pub name: &'static str, | ||
| pub display_name: &'static str, | ||
| pub description: &'static str, | ||
| pub default_enabled: bool, |
There was a problem hiding this comment.
The PR description mentions adding per-extension version and version-driven migrations, but the platform extension definition/config only adds display_name (no version field), so migrations are currently based on field diffs; either implement the version field end-to-end (schema + config + migration logic) or update the PR description to match the actual behavior.
| if needs_save { | ||
| save_extensions_map(extensions_map.clone()); | ||
| } |
There was a problem hiding this comment.
get_extensions_map() now writes to disk during reads; if save_extensions_map fails, the migration silently won’t persist and callers still proceed with the in-memory map, so consider returning/propagating a Result (or at least surfacing a warning/error) so migration failures aren’t hidden.
|
@alexhancock I was actually thinking that this could live in the config load itself. Then any time we need to change the config structure we can do it here. WDYT? |
92f9e59 to
61cae9f
Compare
| if let Err(e) = self.save_values(values.clone()) { | ||
| tracing::warn!("Failed to save migrated config: {}", e); |
There was a problem hiding this comment.
Config::load() may write the migrated config back to disk without taking self.guard, which can race with concurrent set_param/delete calls and cause lost updates; take the same mutex around the migration save (or route through a guarded write path).
| if let Err(e) = self.save_values(values.clone()) { | |
| tracing::warn!("Failed to save migrated config: {}", e); | |
| // Persist migrated config under the same guard used by other writers | |
| match self.guard.lock() { | |
| Ok(_guard) => { | |
| if let Err(e) = self.save_values(values.clone()) { | |
| tracing::warn!("Failed to save migrated config: {}", e); | |
| } | |
| } | |
| Err(e) => { | |
| tracing::warn!("Failed to acquire config lock for migrated save: {}", e); | |
| } |
| pub fn run_migrations(config: &mut Mapping) -> bool { | ||
| let mut changed = false; | ||
| changed |= migrate_platform_extensions(config); | ||
| changed | ||
| } |
There was a problem hiding this comment.
The PR description and example config show a per-extension version field used to drive migrations, but this PR only adds display_name (no version field in ExtensionConfig, OpenAPI, or the migration logic); either implement the versioned migration mechanism or update the PR description/screenshots to match the actual behavior.
61cae9f to
2f4a46d
Compare
2f4a46d to
ea693e8
Compare
ea693e8 to
d7fcf15
Compare
d7fcf15 to
d1979b1
Compare
* main: docs: usage data collection (#6822) feat: platform extension migrator + code mode rename (#6611) feat: CLI flag to skip loading profile extensions (#6780) Swap canonical model from openrouter to models.dev (#6625) Hook thinking status (#6815) Fetch new skills hourly (#6814) copilot instructions: Update "No prerelease docs" instruction (#6795) refactor: centralize audience filtering before providers receive messages (#6728) update doc to remind contributors to activate hermit and document minimal npm and node version (#6727) nit: don't spit out compaction when in term mode as it fills up the screen (#6799) fix: correct tool support detection in Tetrate provider model fetching (#6808) Session manager fixes (#6809) fix(desktop): handle quoted paths with spaces in extension commands (#6430) fix: we can default gooseignore without writing it out (#6802) fix broken link (#6810) docs: add Beads MCP extension tutorial (#6792) feat(goose): add support for AWS_BEARER_TOKEN_BEDROCK environment variable (#6739)
* 'main' of github.com:block/goose: Fix: Small update UI settings prompt injection (#6830) Remove autogenerated .gooseignore files that don't belong in repo (#6824) Fix case-insensitive matching for builtin extension names (#6825) docs: cli newline keybinding (#6823) Update version to 1.22.0 (#6821) Refactor: move persisting extension to session outside of route (#6685) acp: load configured extensions and refactor tests (#6803) docs: usage data collection (#6822) feat: platform extension migrator + code mode rename (#6611) feat: CLI flag to skip loading profile extensions (#6780)
Summary
At present, it's impossible to rename or edit platform extensions in the stored config. For three reasons:
config.yamlif the platform extension is already thereThis PR introduces two new values that make it possible to update the config stored in a user's configuration file for platform extensions -
display_name(shows in the UI, and is not coupled to the name identifier) andversionwhich can be bumped when we have made config updates to an extension to have it take effect in the config file.It also introduces the first migration which is to rename "Code Execution" to "Code Mode" per the request in #6581
This can also be used in the future to rename or edit other extension config (descriptions, default enabled value, etc)
Type of Change
AI Assistance
Goose
Testing
Manually reset config file after changes to previous default state, then launched new version of goose and observed new config file and updated values in the UI
Related Issues
#6581
Screenshots/Demos (for UX changes)
Before:
config.yamlAfter:
config.yamlUpdated Name and Description for the Code Mode extension in Desktop: