fix(goose): propagate session_id across providers and MCP #6584
+1,445
−604
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| } | ||
|
|
||
| mock_server | ||
| /// Calling this ensures incidental requests that might error asynchronously, such as |
Collaborator
Author
There was a problem hiding this comment.
this is a very important part of ACP tests as it tracks any session ID mismatch including no session ID. if any propagation breaks in the future, and it is reproducible by ACP, this will be the tripwire
Contributor
There was a problem hiding this comment.
Pull request overview
This PR propagates explicit session_id parameters through provider and MCP requests, replacing task-local context with explicit parameter passing. This ensures coherent session IDs everywhere, fixing issues with MCP sampling in health checks and session rename requests.
Changes:
- Added
session_idparameter to all Provider trait methods and implementations - Removed
McpMetastruct in favor of directsession_idparameter in MCP client trait - Updated API client to inject session IDs into HTTP headers explicitly
- Added comprehensive test coverage via ACP integration tests
Reviewed changes
Copilot reviewed 60 out of 61 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
crates/goose/src/providers/base.rs |
Added session_id parameter to Provider trait methods (complete, stream, etc.) |
crates/goose/src/providers/api_client.rs |
Updated to require session_id in request builders, removed task-local lookup |
crates/goose/src/agents/mcp_client.rs |
Removed McpMeta, added session resolution logic with fallbacks, updated all MCP methods |
crates/goose/src/agents/extension_manager.rs |
Propagated session_id through all extension operations |
crates/goose/src/session/session_manager.rs |
Updated generate_session_name to pass session_id |
crates/goose/src/scheduler.rs |
Removed session_context wrapper (session_id now in SessionConfig) |
crates/goose-acp/tests/common.rs |
Added session ID validation fixtures for integration tests |
| Provider implementations (20+ files) | Updated all providers to accept and propagate session_id parameter |
| Test files | Updated all tests to provide explicit session_id values |
Collaborator
codefromthecrypt
force-pushed
the
session-propagation
branch
from
f02e430 to
2d1ff26
Compare
codefromthecrypt
force-pushed
the
session-propagation
branch
from
2d1ff26 to
d20012b
Compare
Signed-off-by: Adrian Cole <adrian@tetrate.io>
codefromthecrypt
force-pushed
the
session-propagation
branch
from
1187bb2 to
df165a6
Compare
michaelneale
approved these changes
Jan 22, 2026
Collaborator
michaelneale
left a comment
michaelneale
left a comment
There was a problem hiding this comment.
this seems necessary and ok - if can work out about live failure (not likely related)
Collaborator
Author
|
the live one must have been a flake. merging this one in as without it sessions are lost through acp and also we have some edge cases even in normal paths (like session rename) which have incoherent session id propagation. |
lifeizhou-ap
added a commit
that referenced
this pull request
Jan 22, 2026
* main: (41 commits) chore: tweak release docs (#6571) fix(goose): propagate session_id across providers and MCP (#6584) increase worker threads for ci (#6614) docs: todo tutorial update (#6613) Added goose doc map md file for goose agent to find relevant doc easily. (#6598) add back goose branding to home (#6617) fix: actually set the working dir for extensions from session (#6612) Multi chat (#6428) Lifei/fixed accumulated token count (#6587) Dont show MCP UI/Apps until tool is approved (#6492) docs: max tokens config (#6596) User configurable templates (#6420) docs: http proxy environment variables (#6594) feat: exclude subagent tool from code_execution filtering (#6531) Fix path for global agent skills (#6591) recipes: add mcp server (#6552) feat(gcp-vertex): add model list with org policy filtering (#6393) chore: encourage extension searching (#6582) blog: mobile apps consolidation and roadmap (#6580) chore: remove unused dependencies in cargo.toml (#6561) ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Propagate explicit
session_idthrough provider and MCP requests, allowing coherent session IDs everywhere we can.We use a fallback for two cases:
This notably fixes session-id propagation such as from servers who do MCP sampling in health checks.
This also fixes session ID propagation in session rename requests originally started in #5624.
Importantly, by backfilling ACP integration hooks, we now can fail any change that breaks session ID propagation.
This is important because before it was subtle, like task locals which might not even be the correct context. For example,
MCP sampling callbacks do not share a task local with a tool call that caused it!
Type of Change
AI Assistance
Testing
Related Issues
Resumes from #5624