-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Propagate session ID in LLM and MCP requests #5165
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Propagate session ID in LLM and MCP requests #5165
Conversation
|
very flexible on this one, too, header naming.. whether it is configurable.. all that! |
jamadeo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems very useful! if you could have a quick look at any redundant comments and pare them down that would be great. also if possible avoid using environment variables in tests, I think this should be possible in your case
|
thanks for the advice! I was AWOL but coming online again now. will touch this up tomorrow UTC+8 |
9ef4f13 to
7629127
Compare
|
ok rebased and also tried to shred the lines in a good way. if I made things worse lemme know |
| @@ -1,5 +1,5 @@ | |||
| pub mod anthropic; | |||
| mod api_client; | |||
| pub mod api_client; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needed to expose this for tests in order to write them without ENV dependencies
|
cool I think the live provider red X is due to it not being on a protected branch. probably safe to review! |
jamadeo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, and yeah, those provider tests aren't configured correctly right now. (they need access to secrets for api keys, but those aren't available on forks)
Adds `goose-session-id` header to HTTP requests and `_meta.goose-session-id` to MCP operations when a session exists. Enables external observability tools like Envoy AI Gateway to correlate requests by session without requiring OpenTelemetry in Goose. Use case: Configure Envoy AI Gateway with `OTEL_AIGW_SPAN_REQUEST_HEADER_ATTRIBUTES=goose-session-id:session.id` to send traces to Arize Phoenix grouped by session. Implementation: - Task-local storage for session context propagation - HTTP header injection in api_client (case-insensitive via reqwest) - MCP metadata injection preserving existing _meta fields (case-insensitive cleanup) - Integration and unit tests for both HTTP and MCP propagation Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Adrian Cole <[email protected]>
213512a to
855b5a8
Compare
|
over to you @jamadeo |
* main: Fixes Gemini API parse issue by converting nullable type arrays to single types in tool schemas (#5530) Troubleshooting diagnostics doc (#5526) fix link to Ollama FAQ (#5531) docs: remove speech-mcp (#5514) fix: adds ProviderRetry to openai provider (#5518) docs: extensions directory minor updates (#5466) Docs/json recipe support (#5492) docs: recipe buttons (#5507) Improve system theme detection and fallback (#5427) [Autovisualiser] remove unnecessary content from mermaid HTML template (#5505) Improve subagents docs (#5484) FIX: prefer linux in WSL and add INSTALL_OS override for CLI (#5215) Propagate session ID in LLM and MCP requests (#5165) feat: YT Short for Canva MCP + goose (#5495)
…se into dkatz/manual-compact-fix-usage * 'dkatz/manual-compact-fix-usage' of github.com:block/goose: (35 commits) Fix image processing (#5544) docs: AI attribution for PRs (#5547) chore(tests/mcp): testing for MCP sampling (#5456) docs: adding HOWTOAI.md (#5533) added configuration content, also added signoff, fix merging issue with another commit by creating a clean branch. removed and closed commits that caused signoff issues. (#5519) Fixes Gemini API parse issue by converting nullable type arrays to single types in tool schemas (#5530) Troubleshooting diagnostics doc (#5526) fix link to Ollama FAQ (#5531) docs: remove speech-mcp (#5514) fix: adds ProviderRetry to openai provider (#5518) docs: extensions directory minor updates (#5466) Docs/json recipe support (#5492) docs: recipe buttons (#5507) Improve system theme detection and fallback (#5427) [Autovisualiser] remove unnecessary content from mermaid HTML template (#5505) Improve subagents docs (#5484) FIX: prefer linux in WSL and add INSTALL_OS override for CLI (#5215) Propagate session ID in LLM and MCP requests (#5165) feat: YT Short for Canva MCP + goose (#5495) Change Recipes Test Script (#5457) ...
* 'main' of github.com:block/goose: (61 commits) [Autovisualiser] remove unnecessary content from mermaid HTML template (#5505) Improve subagents docs (#5484) FIX: prefer linux in WSL and add INSTALL_OS override for CLI (#5215) Propagate session ID in LLM and MCP requests (#5165) feat: YT Short for Canva MCP + goose (#5495) Change Recipes Test Script (#5457) Goose recover (#5450) don't start the default provider (#5351) keep the order of keys in config.yaml (#5468) Removed drafts and agentIsReady in ChatInput (#5366) nextcamp - fix session resume when navigating back to chat in sidebar (#5370) feat/fix: set optional config params, and don't overwrite unset secrets (#5325) Stringly typed config (#5463) Fix: Compaction client <-> server sync (#5481) docs: recipe activity parameter substitution (#5462) only run fork on branch PRs (#5461) docs: video on goose with apify mcp (#5472) Clear windows and fix build failure (#5452) Add menu option for setting window always on top (#5429) Delete environment variable (#5479) ...
Signed-off-by: fbalicchia <[email protected]>
Wrap session name generation task in session_context to inject session ID header into LLM requests via api_client.rs. Without this, rename task requests lacked the goose-session-id header despite having the session ID as a parameter. Completes work started in block#5165. ```mermaid sequenceDiagram participant Agent participant ReplyInternal participant NamingTask participant ApiClient participant LLM Agent->>ReplyInternal: reply(session_id=abc-123) rect rgb(255, 200, 200) Note over ReplyInternal,LLM: Before: rename task missing session header ReplyInternal->>NamingTask: tokio::spawn(without context) NamingTask->>ApiClient: generate_session_name() Note over ApiClient: current_session_id() → None ApiClient->>LLM: POST /chat (no goose-session-id) LLM-->>NamingTask: "Generated Name" end Note over NamingTask: Testing requires sleep-based timing ``` ```mermaid sequenceDiagram participant Agent participant ReplyInternal participant NamingTask participant ApiClient participant LLM Agent->>ReplyInternal: reply(session_id=abc-123) rect rgb(191, 223, 255) Note over ReplyInternal,LLM: After: session ID injected in headers ReplyInternal->>NamingTask: tokio::spawn(with_session_id(abc-123)) NamingTask->>ApiClient: generate_session_name() Note over ApiClient: current_session_id() → abc-123 ApiClient->>LLM: POST /chat (goose-session-id: abc-123) LLM-->>NamingTask: "Generated Name" end Note over ReplyInternal: Process messages... ReplyInternal->>NamingTask: await handle NamingTask-->>ReplyInternal: complete Note over Agent: Task completion verified (deterministic) ``` Unit test matrix covers success, error, and panic cases. Integration test verifies goose-session-id header appears in rename task's LLM requests without timing dependencies. Sets foundation for future trace context propagation tests. Signed-off-by: Adrian Cole <[email protected]>
Wrap session name generation task in session_context to inject session ID header into LLM requests via api_client.rs. Without this, rename task requests lacked the goose-session-id header despite having the session ID as a parameter. Follows up block#5165 by extending session ID propagation to the rename task. ```mermaid sequenceDiagram participant Agent participant ReplyInternal participant NamingTask participant ApiClient participant LLM Agent->>ReplyInternal: reply(session_id=abc-123) rect rgb(255, 200, 200) Note over ReplyInternal,LLM: Before: rename task missing session header ReplyInternal->>NamingTask: tokio::spawn(without context) NamingTask->>ApiClient: generate_session_name() Note over ApiClient: current_session_id() → None ApiClient->>LLM: POST /chat (no goose-session-id) LLM-->>NamingTask: "Generated Name" end Note over NamingTask: Testing requires sleep-based timing ``` ```mermaid sequenceDiagram participant Agent participant ReplyInternal participant NamingTask participant ApiClient participant LLM Agent->>ReplyInternal: reply(session_id=abc-123) rect rgb(191, 223, 255) Note over ReplyInternal,LLM: After: session ID injected in headers ReplyInternal->>NamingTask: tokio::spawn(with_session_id(abc-123)) NamingTask->>ApiClient: generate_session_name() Note over ApiClient: current_session_id() → abc-123 ApiClient->>LLM: POST /chat (goose-session-id: abc-123) LLM-->>NamingTask: "Generated Name" end Note over ReplyInternal: Process messages... ReplyInternal->>NamingTask: await handle NamingTask-->>ReplyInternal: complete Note over Agent: Task completion verified (deterministic) ``` Unit test matrix covers success, error, and panic cases. Integration test verifies goose-session-id header appears in rename task's LLM requests without timing dependencies. Sets foundation for future trace context propagation tests. Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Blair Allan <[email protected]>
Summary
Adds
goose-session-idheader to HTTP requests and_meta.goose-session-idto MCP operations when a session exists. Enables external observability tools like Envoy AI Gateway to correlate requests by session without requiring OpenTelemetry in Goose.Implementation:
Example use case: Configure Envoy AI Gateway with
OTEL_AIGW_SPAN_REQUEST_HEADER_ATTRIBUTES=goose-session-id:session.idto send traces to Arize Phoenix grouped by session.Type of Change
Testing
added tests including an integrated one
Related Issues
Relates to envoyproxy/ai-gateway#1221 which asked for this in other agent frameworks