Skip to content

fix: per session working dir isolation#6920

Merged
zanesq merged 2 commits intomainfrom
zane/working-dir-mcp
Feb 3, 2026
Merged

fix: per session working dir isolation#6920
zanesq merged 2 commits intomainfrom
zane/working-dir-mcp

Conversation

@zanesq
Copy link
Collaborator

@zanesq zanesq commented Feb 3, 2026

Problem

Working directory was not isolated per chat session. Changing the working directory in one
session would affect other sessions because:

  1. Frontend: A module-level global currentWorkingDir in workingDir.ts was shared
    across all sessions
  2. Backend: std::env::set_var("GOOSE_WORKING_DIR", ...) set a process-global
    environment variable that all sessions read from

Solution

Pass the working directory per-request through MCP metadata instead of using global state.

Frontend Changes

  • ui/desktop/src/utils/workingDir.ts: Removed currentWorkingDir global and
    setCurrentWorkingDir() function
  • ui/desktop/src/components/bottom_menu/DirSwitcher.tsx: Removed call to
    setCurrentWorkingDir()

Backend Changes

Core plumbing:

  • session_context.rs: Added WORKING_DIR_HEADER constant for MCP metadata key
  • mcp_client.rs:
    • Added working_dir parameter to McpClientTrait::call_tool()
    • Created inject_session_context_into_extensions() and inject_session_context_into_request()
      to inject both session_id and working_dir into MCP request metadata
    • Updated send_request_with_context() to handle working directory injection

Extension manager:

  • extension_manager.rs:
    • Added working_dir: Option<&Path> parameter to dispatch_tool_call()
    • Removed std::env::set_var("GOOSE_WORKING_DIR", ...) for builtin extensions
    • Passes working_dir through to McpClient::call_tool()

Developer extension:

  • rmcp_developer.rs:
    • Added extract_working_dir_from_meta() helper to read working_dir from MCP context
    • Modified shell() to extract working_dir from request metadata
    • Modified execute_shell_command() to accept working_dir: Option<PathBuf> parameter
      instead of reading from environment variable

Call sites updated:

  • agent.rs - passes Some(session.working_dir.as_path())
  • goose-server/routes/agent.rs - passes None (direct API calls)
  • code_execution_extension.rs - passes None
  • All McpClientTrait implementations updated with _working_dir parameter
  • All test mocks updated

Result

Each session's working directory is now passed through the request chain and isolated
from other sessions. No more global state pollution.

fixes #6909

@zanesq zanesq requested review from DOsinga, baxen and jamadeo February 3, 2026 17:28
@zanesq zanesq changed the title isolate working_dir per session in mcp _meta fix: per session working dir isolation Feb 3, 2026
.dispatch_tool_call(
&session.id,
tool_call.clone(),
Some(session.working_dir.as_path()),
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe dispatch_tool_call should just take a reference to the session itself? now that we're passing two fields from it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I looked into this and its a pretty large refactor because not all call sites have access to a full session and it would add additional overhead to create a session and/or mock a fake one etc. Gonna punt on this for now and we can come back to it later..

Copy link
Collaborator

Choose a reason for hiding this comment

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

that makes sense - thanks for looking into it

@zanesq zanesq added this pull request to the merge queue Feb 3, 2026
Merged via the queue into main with commit aa356bd Feb 3, 2026
18 checks passed
@zanesq zanesq deleted the zane/working-dir-mcp branch February 3, 2026 18:25
@DOsinga
Copy link
Collaborator

DOsinga commented Feb 4, 2026

this looks a lot nicer than what we have now, thanks for doing this.

but a propos now, shouldn't this be matched by the deletion of a bunch of corresponding lines?

pub async fn add_extension( still takes a working_dir for example

@zanesq
Copy link
Collaborator Author

zanesq commented Feb 4, 2026

The working_dir parameter in add_extension is used for setting the working directory of child processes effective_working_dir so its still needed there but agreed it could be better organized by computing effective_working_dir lazily only where needed instead. I'll do a follow up PR 👍

@DOsinga
Copy link
Collaborator

DOsinga commented Feb 4, 2026

thanks @zanesq - you are right that's still needed I guess

@zanesq
Copy link
Collaborator Author

zanesq commented Feb 4, 2026

np here is the follow up just making it a bit clearer for now why we do this #6958

stebbins pushed a commit to stebbins/goose that referenced this pull request Feb 4, 2026
Signed-off-by: Harrison <hcstebbins@gmail.com>
kuccello pushed a commit to kuccello/goose that referenced this pull request Feb 7, 2026
Tyler-Hardin pushed a commit to Tyler-Hardin/goose that referenced this pull request Feb 11, 2026
Tyler-Hardin pushed a commit to Tyler-Hardin/goose that referenced this pull request Feb 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Working directory shared across sessions via process-wide env var

3 participants

Comments