fix: reverting a change to try fix code mode#6622
Conversation
There was a problem hiding this comment.
Pull request overview
This PR reverts changes from #6612 that added working directory parameter passing to extension loading, in order to fix live test failures with code mode. The revert removes the add_extension_with_working_dir method and its working_dir parameter throughout the codebase, simplifying the API back to just add_extension.
Changes:
- Removed
add_extension_with_working_dirmethod and working_dir parameter passing from agent and extension manager - Reverted test to use simpler
add_extensionAPI - Removed
GOOSE_WORKING_DIRenvironment variable setting for builtin extensions
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crates/goose/tests/mcp_integration_test.rs | Reverts test to use add_extension instead of add_extension_with_working_dir |
| crates/goose/src/agents/extension_manager_extension.rs | Removes working_dir parameter from extension addition |
| crates/goose/src/agents/extension_manager.rs | Removes add_extension_with_working_dir method, simplifies to add_extension, removes GOOSE_WORKING_DIR setting for builtins |
| crates/goose/src/agents/agent.rs | Removes add_extension_with_working_dir wrapper and working_dir parameter passing in load_extensions_from_session |
| /// Resolve the working directory for an extension. | ||
| /// Falls back to current_dir when working_dir is not available. | ||
| async fn resolve_working_dir(&self) -> PathBuf { | ||
| // Fall back to current_dir - working_dir is passed through the call chain from session |
There was a problem hiding this comment.
The comment claims "working_dir is passed through the call chain from session" but the function always returns std::env::current_dir() without any session involvement. Either remove this misleading part of the comment or update the implementation to actually use the session's working_dir.
| // Fall back to current_dir - working_dir is passed through the call chain from session | |
| // Fall back to current_dir; return an empty PathBuf if it cannot be determined |
| // Resolve working_dir: explicit > current_dir | ||
| let effective_working_dir = | ||
| working_dir.unwrap_or_else(|| std::env::current_dir().unwrap_or_default()); | ||
| // Resolve working_dir: session > current_dir |
There was a problem hiding this comment.
The comment says "Resolve working_dir: session > current_dir" but the implementation doesn't access session at all - it only uses current_dir(). Update the comment to accurately reflect the current behavior.
1 participant
seem to be Live test failures with code mode since #6612
this attempts to roll that back to see if related