-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Add readonly and toolset request handlers #1858
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
Conversation
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.
Pull request overview
This PR adds support for HTTP route handlers that enable readonly mode and toolset filtering via URL paths (/readonly, /x/{toolset}, /x/{toolset}/readonly). It refactors the configuration extraction logic from directly reading headers to using context values, with route-based middleware taking precedence over header-based configuration.
Changes:
- Added context helper functions for storing and retrieving readonly mode and toolset configuration
- Introduced middleware to extract MCP-related headers into context and route-specific middleware for URL-based configuration
- Refactored inventory filtering to primarily read from context instead of headers, maintaining backward compatibility with header-based configuration
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/context/request.go | New context helper functions for readonly and toolsets state management |
| pkg/http/middleware/request_config.go | New middleware to extract MCP headers into request context |
| pkg/http/headers/parse.go | New utility function to parse comma-separated header values |
| pkg/http/handler.go | Added route registration for readonly/toolset paths, route middleware, and refactored inventory filtering to use context |
| pkg/http/handler_test.go | Comprehensive tests for inventory filtering logic with various context and header combinations |
| // WithToolsets adds the active toolsets to the context | ||
| func WithToolsets(ctx context.Context, toolsets []string) context.Context { | ||
| return context.WithValue(ctx, toolsetsCtxKey{}, toolsets) |
Copilot
AI
Jan 23, 2026
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.
The WithToolsets function stores a slice directly in the context without copying it. If the caller modifies the slice after calling WithToolsets, it could lead to unexpected behavior or race conditions since the same slice reference is stored. Consider documenting that the slice should not be modified after being passed, or make a defensive copy of the slice before storing it in the context.
| // WithToolsets adds the active toolsets to the context | |
| func WithToolsets(ctx context.Context, toolsets []string) context.Context { | |
| return context.WithValue(ctx, toolsetsCtxKey{}, toolsets) | |
| // WithToolsets adds the active toolsets to the context. | |
| // The provided slice is defensively copied to avoid unexpected mutations. | |
| func WithToolsets(ctx context.Context, toolsets []string) context.Context { | |
| copied := append([]string(nil), toolsets...) | |
| return context.WithValue(ctx, toolsetsCtxKey{}, copied) |
Summary
Adds support for
/readonly,/x/{toolset}and/x/{toolset}/readonly.Why
What changed
withReadonlyandwithToolsetroute middleware/readonly,/x/{toolset}and/x/{toolset}/readonlyroutesWithRequestConfigmiddleware to extract MCP headers into context and refactoredInventoryFiltersForRequestto primarily read from context instead of headersMCP impact
Prompts tested (tool changes only)
Security / limits
Tool renaming
deprecated_tool_aliases.goNote: if you're renaming tools, you must add the tool aliases. For more information on how to do so, please refer to the official docs.
Lint & tests
./script/lint./script/testDocs