Add PATH detection back to developer extension#7161
Conversation
There was a problem hiding this comment.
Pull request overview
This PR restores user-PATH detection for the in-process developer builtin extension in goose-server, addressing the regression described in #7153 when the extension stopped running as a child process.
Changes:
- Adds a custom
spawn_developerbuiltin spawn function that configuresDeveloperServerto extendPATHvia the user’s shell and uses a.bash_envfile. - Overrides the default
"developer"entry in the builtin extension registry duringAppState::new()initialization.
| register_builtin_extensions(goose_mcp::BUILTIN_EXTENSIONS.clone()); | ||
| register_builtin_extension("developer", spawn_developer); | ||
|
|
There was a problem hiding this comment.
This only overrides the built-in "developer" spawn fn in goose-server; other entrypoints that call register_builtin_extensions(goose_mcp::BUILTIN_EXTENSIONS.clone()) (e.g. crates/goose-cli/src/cli.rs and crates/goose-acp/src/server.rs) will still use the default DeveloperServer without PATH extension, so the PATH regression may persist outside goose-server—consider applying the same override there or centralizing a shared helper for a "developer-with-shell-PATH" spawn fn.
There was a problem hiding this comment.
this is intentional, we don't do this for goose-acp and goose-cli
DOsinga
left a comment
DOsinga
left a comment
There was a problem hiding this comment.
we should do a smoke-test with a reliable provider that does more specific scenarions - that could catch this, no?
michaelneale
left a comment
michaelneale
left a comment
There was a problem hiding this comment.
good catch - this seems to keep happening?
codefromthecrypt
left a comment
codefromthecrypt
left a comment
There was a problem hiding this comment.
thanks for putting this back, sorry about the broken eggs.
| pub extension_loading_tasks: ExtensionLoadingTasks, | ||
| } | ||
|
|
||
| fn spawn_developer(r: tokio::io::DuplexStream, w: tokio::io::DuplexStream) { |
There was a problem hiding this comment.
tempted to thread through the config dir, so that it is testable. can take that in another PR, if you like
* main: fix text editor view broken (#7167) docs: White label guide (#6857) Add PATH detection back to developer extension (#7161) docs: pin version in ci/cd (#7168) Desktop: - No Custom Headers field for custom OpenAI-compatible providers (#6681) feat: edit model and extensions of a recipe from GUI (#6804) feat: MCP support for agentic CLI providers (#6972)
* origin/main: (33 commits) fix: replace panic with proper error handling in get_tokenizer (#7175) Lifei/smoke test for developer (#7174) fix text editor view broken (#7167) docs: White label guide (#6857) Add PATH detection back to developer extension (#7161) docs: pin version in ci/cd (#7168) Desktop: - No Custom Headers field for custom OpenAI-compatible providers (#6681) feat: edit model and extensions of a recipe from GUI (#6804) feat: MCP support for agentic CLI providers (#6972) docs: keyring fallback to secrets.yaml (#7165) feat: load provider/model specified inside the recipe config (#6884) fix ask-ai bot hitting tool call limits (#7162) fix flatpak icon (#7154) [docs] Skills Marketplace UI Improvements (#7158) More no-window flags (#7122) feat: Allow overriding default bat themes using environment variables (#7140) Make the system prompt smaller (#6991) Pre release script (#7145) Spelling (#7137) feat(mcp): upgrade rmcp to 0.15.0 and advertise MCP Apps UI extension capability (#6927) ...
|
A test for the developer PATH that emulates the real-world use case is tricky, because launching anything from the terminal is always going to inherit the PATH. So I think the way to do this would be to launch goose with a constrained PATH, missing something you would expect the interactive shell to have, and then check that the developer tool does inherit it. I'll look into adding something like that. |
…provenance * origin/main: (68 commits) Upgraded npm packages for latest security updates (#7183) docs: reasoning effort levels for Codex provider (#6798) Fix speech local (#7181) chore: add .gooseignore to .gitignore (#6826) Improve error message logging from electron (#7130) chore(deps): bump jsonwebtoken from 9.3.1 to 10.3.0 (#6924) docs: standalone mcp apps and apps extension (#6791) workflow: auto-update cli-commands on release (#6755) feat(apps): Integrate AppRenderer from @mcp-ui/client SDK (#7013) fix(MCP): decode resource content (#7155) feat: reasoning_content in API for reasoning models (#6322) Fix/configure add provider custom headers (#7157) fix: handle keyring fallback as success (#7177) Update process-wrap to 9.0.3 (9.0.2 is yanked) (#7176) feat: support extra field in chatcompletion tool_calls for gemini openai compat (#6184) fix: replace panic with proper error handling in get_tokenizer (#7175) Lifei/smoke test for developer (#7174) fix text editor view broken (#7167) docs: White label guide (#6857) Add PATH detection back to developer extension (#7161) ... # Conflicts: # .github/workflows/nightly.yml
4 participants
fixes #7153
it was lost in the #6392 refactor. Maybe there's a not-too-expensive test for this.