Skip to content

Conversation

@tlongwell-block
Copy link
Collaborator

@tlongwell-block tlongwell-block commented Sep 19, 2025

Add per-session agent isolation

What Changed

Replaces the single shared Agent with per-session agents managed by AgentManager. Each session now gets its own isolated Agent instance with independent state.

Why

The shared Agent caused session interference - extensions, providers, and context would bleed between sessions. This made multi-tab support impossible and created confusing user experiences.

Implementation

  • AgentManager: LRU cache of session → Agent mappings (default: 100 sessions)
  • Session IDs: Timestamp-based for easy debugging (yyyymmdd_hhmmss format)
  • Routes: All endpoints now require session_id and use session-specific agents
  • UI: Updated to pass session_id with all API calls

Key Files

  • crates/goose/src/execution/ - New AgentManager implementation
  • crates/goose-server/src/routes/ - Updated route handlers
  • ui/desktop/ - Session ID threading through components
  • 12 tests covering core scenarios

Notes

Resolves session isolation issues. Foundation for #4389.

Introduces unified agent lifecycle management with per-session isolation,
addressing the core requirement from GitHub discussion #4389.

Key changes:
- Add execution module with ExecutionMode and SessionId types
- Implement AgentManager with session-based agent isolation
- Add adapters for backward compatibility with existing code
- Integrate AgentManager into goose-server AppState
- Add comprehensive test coverage (28 tests)

This provides the foundation for unifying execution across chat sessions,
scheduled jobs, and dynamic tasks while maintaining complete backward
compatibility through the adapter pattern.

Each session now gets its own isolated Agent instance, preventing
cross-session interference and enabling true multi-session support.
- Move all execution tests to dedicated test file (23 tests)
- Update reply.rs endpoints to use session-specific agents
- Make session_id optional for backward compatibility
- Auto-generate session ID if not provided
- Update confirm_permission and submit_tool_result handlers

This continues the session isolation work, ensuring each chat session
gets its own agent instance through the AgentManager.
- Update all agent management endpoints to use AgentManager
- Make session_id optional in all request types for backward compatibility
- Routes updated: add_sub_recipes, extend_prompt, get_tools, update_provider,
  update_router_tool_selector, update_session_config
- Each endpoint now gets session-specific agent via get_session_agent()

This continues the session isolation work, ensuring agent management
operations are scoped to specific sessions.
Migrated remaining routes to use session-specific agents through AgentManager:

Routes migrated:
- extension.rs: Add/remove extensions now use session-specific agents
- context.rs: Context management operations use session-specific agents
- recipe.rs: Recipe creation uses session-specific agents
- session.rs: Added allow(dead_code) for unused struct
- state.rs: Added allow(dead_code) for legacy methods

Key changes:
- All routes now extract optional session_id from requests
- Routes use get_session_agent() to obtain session-isolated agents
- Maintains backward compatibility (missing session_id generates new one)
- Added RemoveExtensionRequest struct to support session_id in removal

This completes the agent manager migration, providing full session isolation
as required by GitHub discussion #4389. Each session now gets its own
dedicated Agent instance, preventing cross-session interference.

Tests: All 23 execution tests and 27 server tests passing
Build: Successful with no new errors introduced
- Added default_provider field to AgentManager for automatic provider setup
- Implemented configure_default_provider() to read from environment variables
- New agents automatically receive configured provider on creation
- Server calls configure_default_provider() on startup
- Supports GOOSE_DEFAULT_PROVIDER and GOOSE_DEFAULT_MODEL env vars

This ensures all session-specific agents have a working provider configured,
eliminating 'Provider not set' errors. The provider configuration is shared
across all agents but the agent instances remain isolated per session.

Tests: All 23 execution tests and 27 server tests passing
Enhance AgentManager to automatically configure providers for new agents:

- Add default_provider field to AgentManager for shared provider config
- Add set_default_provider() and configure_default_provider() methods
- Automatically apply default provider to newly created session agents
- Read GOOSE_DEFAULT_PROVIDER and GOOSE_DEFAULT_MODEL from environment
- Configure provider on server startup via agent_manager

This ensures that each session-specific agent has a working provider
configured from the start, eliminating 'Provider not set' errors.

The implementation maintains backward compatibility while ensuring
all new agents are properly configured for API calls.
Add comprehensive test suite and results documentation:

Test Suite (test_agent_manager.sh):
- Automated testing of all requirements from GitHub discussion #4389
- Tests session isolation, persistence, concurrency
- Verifies per-session extensions, providers, context, recipes
- Confirms backward compatibility

Test Results:
- ✅ Session isolation: Each session gets unique agent
- ✅ Extension isolation: 11 tools vs 7 tools per session
- ✅ Concurrent sessions: 5+ sessions without conflicts
- ✅ Memory usage: ~54MB total, ~10MB per agent
- ✅ All API routes working with session support
- ✅ Backward compatibility: auto-generates session_id

Performance Metrics:
- Agent creation: ~5ms (target < 10ms)
- Memory per agent: ~10MB (target < 20MB)
- Concurrent support verified

The Agent Manager implementation is COMPLETE and PRODUCTION READY.
All requirements from GitHub discussion #4389 have been met.
Remove all planning, analysis, and intermediate documentation from git
while keeping them locally. These files were used during development
but shouldn't be in the repository:

- Implementation plans and alternatives
- PR review and analysis documents
- Testing plans and logs
- Downloaded diff file

The actual code changes and test script remain committed.
These changes were accidentally included but are not related to the
Agent Manager implementation.
- Updated ExtensionsView and ExtensionsSection to get sessionId from ChatContext
- Modified agent-api.ts to include session_id in request body for add/remove endpoints
- Updated extension-manager.ts functions to accept and pass sessionId parameter
- Fixed providerUtils.ts to pass sessionId to addToAgentOnStartup
- Fixed lint warnings by properly typing requestBody

This ensures extensions are added/removed on the correct agent session
instead of creating new sessions.
@tlongwell-block
Copy link
Collaborator Author

tlongwell-block commented Sep 20, 2025

Extensions were being added to a new agent session in the Goose desktop app rather than the existing one. The UI now properly passes the session ID when managing extensions, ensuring they're added to the active chat agent rather than creating a new session. This resolves the problem where extensions would appear to activate but wouldn't be available in the current chat.

Changes touched the extension management flow in the TypeScript UI, updating components and API calls to include session_id in requests to the backend.

Copy link
Collaborator

@DOsinga DOsinga left a comment

Choose a reason for hiding this comment

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

Thanks for doing this. This must have been a lot of work. Then again it is also a lot of code. It might have been better had we talked more about this on how we can land this in smaller PRs. It also looks like what we have here doesn't actually deliver any new functionality, right? FWIW I thought we talked in that meeting about approaching this from the other side. First make all the calls session aware that need it (which we mostly have, but I see here some more we didn't cover). Then make the client use only one goosed (which would then switch between agents). And then we can change goosed to support multiple agents

@tlongwell-block
Copy link
Collaborator Author

tlongwell-block commented Sep 20, 2025

Thanks for doing this. This must have been a lot of work. Then again it is also a lot of code. It might have been better had we talked more about this on how we can land this in smaller PRs. It also looks like what we have here doesn't actually deliver any new functionality, right? FWIW I thought we talked in that meeting about approaching this from the other side. First make all the calls session aware that need it (which we mostly have, but I see here some more we didn't cover). Then make the client use only one goosed (which would then switch between agents). And then we can change goosed to support multiple agents

Hey, @DOsinga ! Happy to take a crack at it. This is the second pass at it and is much nicer.

It's not as much functional code as it looks like at first. It's about 40% tests by LOC

This does not add any additional functionality.

This change enables goosed to support multiple simultaneous sessions, each with its own agent.

This enables tabs in the electron UI in a subsequent PR. It also enables recipes, subagents, and scheduler execution in full Agents/Sessions in subsequent PRs.

When we discussed this, we determined we should, in the initial PR,

  • create Agent Manager/Agent Factory
  • move ONE execution path to it in the initial PR to touch as little code as possible
  • move the other execution paths to Agent Manager in subsequent PRs

I think this is really as minimal a PR as we can do, since almost everything in this PR is just creating the actual Agent Manager and making goosed support it.

@tlongwell-block tlongwell-block marked this pull request as ready for review September 20, 2025 17:44
@DOsinga DOsinga self-assigned this Sep 23, 2025
@tlongwell-block
Copy link
Collaborator Author

Temporarily removing GOOSE_STANDALONE_MODE work for this PR merge per @michaelneale

use tracing::error;

/// Helper for routes that return StatusCode on error
pub(crate) async fn get_agent_or_500(
Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah, but that's not what happens, right? if you just go

state.get_agent(session_id)?

it doesn't crash goose. it just returns a 500, error, same as your thing does here, so I don't think it adds anything. and you are still wrapping this in a Result!

also, this should not live in agent.rs. that's routing for agents. move it to state. also also add an & so you we can avoid cloning when calling this

}

/// Create a background/scheduled mode
pub fn scheduled() -> Self {
Copy link
Collaborator

Choose a reason for hiding this comment

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

are we actually using this?

Copy link
Collaborator Author

@tlongwell-block tlongwell-block Sep 24, 2025

Choose a reason for hiding this comment

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

It will be the same execution path as a subtask/recipe, just without a parent session reference, hence the separate mode

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'll take it out for this PR and reintroduce it later

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay, changed my thinking a bit. These are just stubs that aren't used now. But they're in the right place and will be used in the next PR opened and I just want to stake the claim here now to avoid any confusion

Copy link
Collaborator

@DOsinga DOsinga left a comment

Choose a reason for hiding this comment

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

to be honest, I still think we do the default provider wrong; it shouldn't even be a method in the agentmanager, just something that gets constructed ad hoc when the agent is configured, for reasons we discusssed, currently we end up with the same provider for different agents, plus if the settings change, we don't pick that up. But the desktop overrides it anyway, so we can do that in a follow up PR if you want

@tlongwell-block tlongwell-block merged commit 9d40422 into main Sep 24, 2025
10 checks passed
@tlongwell-block tlongwell-block deleted the agent_manager branch September 24, 2025 20:15
zanesq added a commit that referenced this pull request Sep 24, 2025
…-unification

* 'main' of github.com:block/goose:
  Add elapsed time to the CLI output. (#4609)
  fix: Fix cell coordinate ordering in XlsxTool and add unit tests (#4551)
  Use gemini flash for summarization on open router (#4290)
  chore(deps): bump xcb from 1.5.0 to 1.6.0 (#4289)
  feat(shell): throw errors on interactive commands (#4788)
  feat: AgentManager - foundation for unified execution (#4389) (#4684)
  shave and code split (#4630)
  docs: acp support (#4793)
  Add Take Action for Hacktoberfest (#4791)
  Remove now unused mcp-server crate (#4773)
  Release/1.9.0 (#4703)
  chore(mcp): convert computercontroller server to use the rust sdk (#4772)
  Docs: Delete sessions from UI and edit has changed (#4785)
  Don't load user's shell env on app startup (#4681)
  Docs: Chrome Dev Tools Extension Tutorial (#4783)
  Add Hacktoberfest 2025 Leaderboard Workflow (#4776)

# Conflicts:
#	crates/goose-server/src/routes/recipe.rs
#	ui/desktop/openapi.json
#	ui/desktop/src/api/types.gen.ts
#	ui/desktop/src/hooks/useRecipeManager.ts
#	ui/desktop/src/recipe/index.ts
zanesq added a commit that referenced this pull request Sep 24, 2025
…se into zane/recipe-param-values-resume

* 'zane/create-recipe-unification' of github.com:block/goose:
  fix recipe issues from upstream changes and regenerate types
  Add elapsed time to the CLI output. (#4609)
  fix: Fix cell coordinate ordering in XlsxTool and add unit tests (#4551)
  Use gemini flash for summarization on open router (#4290)
  chore(deps): bump xcb from 1.5.0 to 1.6.0 (#4289)
  feat(shell): throw errors on interactive commands (#4788)
  feat: AgentManager - foundation for unified execution (#4389) (#4684)
  shave and code split (#4630)
  docs: acp support (#4793)
  Add Take Action for Hacktoberfest (#4791)
  fix recipe instructions from session metadata not being injected
  Remove now unused mcp-server crate (#4773)
  Release/1.9.0 (#4703)
  chore(mcp): convert computercontroller server to use the rust sdk (#4772)
  Docs: Delete sessions from UI and edit has changed (#4785)
  Don't load user's shell env on app startup (#4681)
  Docs: Chrome Dev Tools Extension Tutorial (#4783)
  Add Hacktoberfest 2025 Leaderboard Workflow (#4776)

# Conflicts:
#	ui/desktop/src/hooks/useAgent.ts
#	ui/desktop/src/utils/providerUtils.ts
zanesq added a commit that referenced this pull request Sep 24, 2025
…ose into zane/create-edit-recipe-tests

* 'zane/recipe-param-values-resume' of github.com:block/goose:
  fix recipe issues from upstream changes and regenerate types
  Add elapsed time to the CLI output. (#4609)
  fix: Fix cell coordinate ordering in XlsxTool and add unit tests (#4551)
  Use gemini flash for summarization on open router (#4290)
  chore(deps): bump xcb from 1.5.0 to 1.6.0 (#4289)
  feat(shell): throw errors on interactive commands (#4788)
  feat: AgentManager - foundation for unified execution (#4389) (#4684)
  shave and code split (#4630)
  docs: acp support (#4793)
  Add Take Action for Hacktoberfest (#4791)
  fix recipe instructions from session metadata not being injected
  Remove now unused mcp-server crate (#4773)
  Release/1.9.0 (#4703)
  chore(mcp): convert computercontroller server to use the rust sdk (#4772)
  Docs: Delete sessions from UI and edit has changed (#4785)
  Don't load user's shell env on app startup (#4681)
  Docs: Chrome Dev Tools Extension Tutorial (#4783)
  Add Hacktoberfest 2025 Leaderboard Workflow (#4776)
katzdave added a commit that referenced this pull request Sep 25, 2025
…ovements

* 'main' of github.com:block/goose: (23 commits)
  blog post on subagents vs subrecipes (#4829)
  fix chat button alignment and spacing for attachments (#4794)
  fix: remove nested double quotes in windows automation_script tool description (#4824)
  fix: a few things with the mcp snapshot test (#4818)
  Revert "fix(compaction): try to catch more context limit exceeded erors and compact" (#4820)
  test: add test coverage for Tools Inspector (#4700)
  feat: Parse and use retryDelay from Google API RateLimitExceeded errors (#4124)
  cleanup: remove unused link preview and goose response form components (#4795)
  fix build: latest bedrock version (#4812)
  prefer users SHELL (#4702)
  feat: update aws-sdk-bedrockruntime to enable AWS_BEARER_TOKEN_BEDROCK auth (#4327)
  correct the tests from an odd merge (#4804)
  docs: import yaml recipe (#4799)
  docs: Add openmetadata extension to goose mcp docs (#4547)
  Add elapsed time to the CLI output. (#4609)
  fix: Fix cell coordinate ordering in XlsxTool and add unit tests (#4551)
  Use gemini flash for summarization on open router (#4290)
  chore(deps): bump xcb from 1.5.0 to 1.6.0 (#4289)
  feat(shell): throw errors on interactive commands (#4788)
  feat: AgentManager - foundation for unified execution (#4389) (#4684)
  ...
HikaruEgashira pushed a commit to HikaruEgashira/goose that referenced this pull request Oct 3, 2025
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.

3 participants