feat: add filter/search/pagination to MCP sessions list API#3824
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
|
|
Warning Review limit reached
More reviews will be available in 24 minutes and 55 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR adds MCPSessionsFilterParams and replaces four unparameterized ListAll* store methods with parameterized List* variants, implements shared RDB filtering/search JOINs, updates the /api/mcp/sessions handler to parse filters, gate store calls, merge and de-duplicate rows, sort deterministically, and return paginated responses; tests and mocks updated accordingly. ChangesMCP Sessions Filterable Listing
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
Confidence Score: 5/5Safe to merge; all user input in SQL filters is parameterized, pagination and de-dup logic is correct, and the breaking interface change is contained to the ConfigStore boundary. The filter-pushdown logic, cross-table de-dup, and pagination are all correct: flowsAllowed() and statusFilterAllowsPending() reliably gate the flow-table calls, the auth-mode filter is applied symmetrically across token and flow queries so de-dup remains consistent, and the stable sort on UTC RFC3339 strings produces deterministic pages. The search JOINs are only added when a search term is present and use hardcoded table/column names with parameterized LIKE values. Test coverage at both the store and handler layers is thorough. No files require special attention. Important Files Changed
Reviews (6): Last reviewed commit: "feat: adds filters for mcp session" | Re-trigger Greptile |
f16dc9a to
f3b68ca
Compare
a2178f8 to
44d3a01
Compare
ee8ef16 to
4618bc3
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@framework/configstore/rdb_mcp_sessions_test.go`:
- Around line 183-193: The test
TestListOauthUserTokens_MatchedUserIDsIgnoredWhenSearchEmpty has the wrong
expected length: seedMCPSessionsFixture seeds 3 OAuth token rows and an empty
Search should return all rows; update the assertion for the result of
store.ListOauthUserTokens called with MCPSessionsFilterParams{MatchedUserIDs:
[]string{"user-42"}} to require.Len(t, got, 3) (replace the current 2) so the
test expectation matches the seeded fixture.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f3e574df-e2d5-4587-b966-eecf6ac843d5
📒 Files selected for processing (6)
framework/configstore/rdb.goframework/configstore/rdb_mcp_sessions_test.goframework/configstore/store.gotransports/bifrost-http/handlers/mcp_sessions.gotransports/bifrost-http/handlers/mcp_sessions_test.gotransports/bifrost-http/lib/config_test.go
🚧 Files skipped from review as they are similar to previous changes (5)
- framework/configstore/store.go
- transports/bifrost-http/handlers/mcp_sessions_test.go
- transports/bifrost-http/handlers/mcp_sessions.go
- transports/bifrost-http/lib/config_test.go
- framework/configstore/rdb.go
44d3a01 to
4385ce1
Compare
4618bc3 to
358bc58
Compare
Merge activity
|
358bc58 to
665483e
Compare
## Summary The four MCP sessions list methods (`ListAllOauthUserTokens`, `ListAllPendingOauthUserSessions`, `ListAllMCPPerUserHeaderCredentials`, `ListAllPendingMCPPerUserHeaderFlows`) previously fetched all rows unconditionally and left all filtering to the handler. This PR pushes filtering down to SQL by introducing a shared `MCPSessionsFilterParams` struct and a reusable `applyMCPSessionFilters` helper, and adds server-side pagination to the merged sessions list endpoint. ## Changes - Renamed the four `ListAll*` store methods to `List*` and added a `MCPSessionsFilterParams` parameter to each. Filters for status, auth mode, MCP client ID, and a case-insensitive search string (matched against MCP client name/ID, user ID, session ID, and virtual key name/ID via LEFT JOIN) are now applied in SQL rather than in the handler. - Introduced `MCPSessionsFilterParams` in `store.go` with documentation explaining why pagination is intentionally omitted from the struct (cross-table de-dup must happen before slicing). - Added `applyMCPSessionFilters` in `rdb.go` as a shared helper that appends the WHERE clauses and search JOINs to a GORM query, parameterised by a `mcpSessionFilterTable` descriptor that carries the table name and the auth-mode column name (which differs between credential tables and flow tables). - Qualified all `ORDER BY` and `WHERE` column references with their table names to avoid ambiguity after the search JOINs are added. - Updated the `list` handler in `mcp_sessions.go` to parse query-string filters (`q`, `status`, `auth_mode`, `mcp_client_id`, `kind`, `limit`, `offset`) via a new `parseMCPSessionsListQuery` function, skip store calls that cannot contribute rows given the filter shape, perform a stable cross-table sort by `created_at DESC` after merging, and return a paginated response with `count`, `total_count`, `limit`, and `offset` fields. - Added `kindAllowed`, `flowsAllowed`, and `statusFilterAllowsPending` helpers on `mcpSessionsListQuery` to cleanly express which store calls to skip. - Updated `MockConfigStore` in `config_test.go` to match the new method signatures. - Added `rdb_mcp_sessions_test.go` with integration-style tests covering no-filter listing, status/auth-mode/MCP-client-ID filtering, search by MCP client name, virtual key name, user ID, and session ID, preload population, and pending-only enforcement. - Added `mcp_sessions_test.go` with unit tests for `parseMCPSessionsListQuery` covering defaults, all parameters, limit capping, invalid limit/offset values, and `kindAllowed` logic. ## Type of change - [ ] Bug fix - [x] Feature - [x] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./framework/configstore/... ./transports/bifrost-http/handlers/... ./transports/bifrost-http/lib/... ``` Validate filtering via the sessions endpoint: ```sh # All sessions (default limit 50) GET /api/mcp/sessions # Filter by status and auth mode with pagination GET /api/mcp/sessions?status=active,needs_reauth&auth_mode=vk&limit=25&offset=0 # Search by MCP client name GET /api/mcp/sessions?q=GitHub # Only token-kind rows GET /api/mcp/sessions?kind=token ``` Expected: response includes `sessions`, `count`, `total_count`, `limit`, and `offset` fields; rows match the supplied filters; `total_count` reflects the full unsliced result. ## Breaking changes - [x] Yes - [ ] No The four `ListAll*` store methods on the `ConfigStore` interface have been renamed and their signatures changed to accept `MCPSessionsFilterParams`. Any external implementations of `ConfigStore` must be updated to match the new signatures. The sessions list API response now includes additional pagination fields (`count`, `total_count`, `limit`, `offset`) alongside `sessions`. ## Related issues N/A ## Security considerations Search JOINs use parameterised LIKE queries; no raw user input is interpolated into SQL. Table and column names in `applyMCPSessionFilters` are hardcoded constants, not user-supplied. DAC scope enforcement remains at the `ScopedDB(ctx)` layer and is unaffected by this change. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
## Summary This PR releases **core v1.5.14**, **framework v1.3.14**, **transports v1.5.6**, and bumps all dependent plugins to their respective `.14` patch versions. It delivers a broad set of new capabilities across MCP authentication, key rotation, OTel metrics, Bedrock/Anthropic compatibility, and UI improvements, alongside a number of targeted bug fixes and refactors. ## Changes - **Direct API Key Header** — Providers can now receive an API key passed directly via a request header (#3817) - **MCP Per-User Auth** — Introduced `MCPCredentialStore` abstraction, per-user MCP credential reconciliation, and a new per-user header auth type with lazy-auth submission flow (#3656, #3702, #3703, #3704, #3705) - **MCP TLS Configuration** — Added configurable TLS (`insecureSkipVerify`, `caCertPem`) for HTTP/SSE MCP client connections (#3779, #3783) - **MCP Sessions Management** — Filter, search, and pagination on the MCP sessions list API and table, plus a `can_reauth` identity gate (#3823, #3824, #3825) - **Key Rotation** — Keys now rotate on 401/402/403 responses; returns `502 upstream_credentials_exhausted` when all keys are permanently exhausted. Added `triggered_rotation` to `KeyAttemptRecord` and tightened `bifrost_key_rotation_events_total` semantics (#3430, #3491) - **OTel Metrics** — Added OTel spec-compatible metrics (backward compatible) with provider cache and semantic cache attributes in metrics export (#3865, #3816) - **Opus 4.8 Support** — System message handling and general compatibility for Opus 4.8 (#3868, #3878) - **Dimension Rankings** — New `GetDimensionRankings` API and dashboard tabs for team, customer, BU, and user rankings (#3766) - **Model Pricing Attributes** — `additional_attributes` field on model pricing rows with management API and UI editor (#3829) - **Prompt Cache Retention** — Added prompt cache retention parameter on responses requests (#3810) - **Tool Call Execution UI** — Inline tool-call execution, stop streaming, bulk execute/submit, and a redesigned tool-call UI (#3837, #3843) - **Sheet Navigation** — Prev/next keyboard navigation and URL state across virtual key, MCP client, and routing rule sheets (#3739, #3740, #3744, #3745) - **Bedrock Tool Name Truncation** — Truncate Bedrock function/tool names to the provider length limit - **Bedrock Guardrails** — Set guardrail config in Bedrock requests built from responses (#3862) - **Anthropic Tool Use** — Default `tool_use` input to `{}` when arguments are absent (#3880) - **Responses Streaming** — Fixed responses stream events (#3838) - **Compat Flow** — Fixed missing parameter parsing on the compat flow (#3881) - **Passthrough API Version** — Set a default API version in passthrough requests as a fallback (#3853) - **Virtual Key Updates** — Avoid overriding optional fields during virtual key update (#3855) - **User-Mode Flows** — Gate user-mode flows on caller `user_id`, skip temp token mint, and unify flow/credential kind filtering for pending flows (#3841, #3859) - **Partial Tool Calls** — Handle partial tool call execution failures and return successful results (#3849) - **URL Query Escaping** — Support escaped characters in URL query parameters (#3826) - **MCP Auth Errors** — Inline banner and retry support for MCP auth-required errors (#3856) - **Renamed Resolvers** — `staticHeadersResolver`/`serverOAuthResolver` renamed to `sharedHeadersResolver`/`sharedOAuthResolver` (#3840) - **Starlark Nested Tool Calls** — Exposed `RunWithPluginPipeline` on `ClientManager` and routed Starlark nested tool calls through the canonical plugin gate (#3794) - **Deferred-Fill OAuth Removed** — Removed deferred-fill user-mode OAuth flow support (#3839) - **Go 1.26.3** — Upgraded toolchain to Go 1.26.3 (#3782) ## Type of change - [x] Bug fix - [x] Feature - [x] Refactor - [ ] Documentation - [x] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [x] Providers/Integrations - [x] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh # Core/Transports go version # should report go1.26.3 go test ./... # UI cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` - Validate MCP per-user auth by configuring a per-user header auth type and confirming credentials are stored and reconciled on virtual key and MCP client changes. - Validate key rotation by triggering a 401/402/403 from an upstream provider and confirming rotation occurs; exhaust all keys and confirm a `502 upstream_credentials_exhausted` is returned. - Validate OTel metrics output includes `provider_cache` and `semantic_cache` attributes. - Validate Bedrock requests with tool names exceeding the provider limit are truncated correctly. - Validate Opus 4.8 system message handling by sending a request with a system message to an Opus 4.8 endpoint. ## Breaking changes - [x] Yes - [ ] No The deferred-fill user-mode OAuth flow has been removed (#3839). Any integrations relying on that flow must migrate to the new per-user credential store approach. The `staticHeadersResolver` and `serverOAuthResolver` identifiers have been renamed to `sharedHeadersResolver` and `sharedOAuthResolver` respectively (#3840); any direct references must be updated. ## Related issues #3817, #3656, #3702, #3703, #3704, #3705, #3779, #3783, #3823, #3824, #3825, #3430, #3491, #3865, #3816, #3868, #3878, #3766, #3829, #3810, #3837, #3843, #3739, #3740, #3744, #3745, #3862, #3880, #3838, #3881, #3853, #3855, #3841, #3859, #3849, #3826, #3856, #3840, #3794, #3839, #3782, #3724, #3814, #3836, #3869, #3886 ## Security considerations - MCP per-user credentials are stored via the new `MCPCredentialStore` abstraction; ensure the backing store is appropriately access-controlled and that credential values are encrypted at rest. - The direct API key header feature passes provider secrets via HTTP headers; ensure TLS is enforced on all ingress paths and that headers are not logged in plaintext. - User-mode flows are now gated on `caller user_id` and temp token minting is skipped where appropriate, reducing the surface for privilege escalation. - TLS configuration for MCP HTTP/SSE connections supports `insecureSkipVerify`; this should only be enabled in controlled environments. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable
## ✨ Features - **Direct API Key Header** - Pass a provider API key directly via request header (#3817) - **MCP Per-User Authentication** - New per-user header auth type with credential storage and lazy-auth submission flow (#3703, #3704, #3705) - **MCP TLS Configuration** - Configurable TLS (insecureSkipVerify, caCertPem) for HTTP/SSE MCP client connections (#3779, #3783) - **MCP Sessions Management** - Filter, search, and pagination on the MCP sessions list API and table, plus a can_reauth identity gate (#3823, #3824, #3825) - **Tool Call Execution UI** - Inline tool-call execution, stop streaming, bulk execute/submit, and a redesigned tool-call UI (#3837, #3843) - **Dimension Rankings Dashboard** - New dashboard tabs for team, customer, BU, and user rankings, backed by a GetDimensionRankings API (#3766) - **Model Pricing Attributes** - additional_attributes on model pricing rows with management API and UI editor (#3829) - **Prompt Cache Retention** - Prompt cache retention parameter on responses requests (#3810) - **Opus 4.8 Support** - System message handling and compatibility for Opus 4.8 (#3878, #3868) - **Key Rotation** - Rotate keys on 401/402/403 and return 502 upstream_credentials_exhausted when all keys are permanently dead (#3491) - **OTel Metrics** - OTel spec compatible metrics plus provider and semantic cache attributes in metrics export (#3865, #3816) - **Sheet Navigation** - Prev/next keyboard navigation and URL state across virtual key, MCP client, and routing rule sheets (#3739, #3740, #3744, #3745) - **Go 1.26.3** - Upgraded toolchain to Go 1.26.3 (#3782) ## 🐞 Fixed - **Bedrock Tool Names** - Truncate Bedrock function/tool names to the provider length limit - **Bedrock Guardrails** - Set guardrail config in Bedrock request built from responses (#3862) - **Anthropic Tool Use** - Default Anthropic tool_use input to {} when arguments are absent (#3880) - **Responses Streaming** - Fixed responses stream events (#3838) - **Compat Flow** - Fixed missing parameter parsing on the compat flow (#3881) - **Passthrough API Version** - Set a default API version in passthrough requests as a fallback (#3853) - **Virtual Key Updates** - Avoid overriding optional fields during virtual key update (#3855) - **User-Mode Flows** - Gate user-mode flows on caller user_id, skip temp token mint, and unify flow/credential kind filtering for pending flows (#3841, #3859) - **Partial Tool Calls** - Handle partial tool call execution failures and return successful results (#3849) - **URL Query Escaping** - Support escaped characters in URL query parameters (#3826) - **MCP Auth Errors** - Inline banner and retry support for MCP auth-required errors (#3856) - **JSON Editor Height** - Cap JSON editor max height at 400px in message views (#3842)

Summary
The four MCP sessions list methods (
ListAllOauthUserTokens,ListAllPendingOauthUserSessions,ListAllMCPPerUserHeaderCredentials,ListAllPendingMCPPerUserHeaderFlows) previously fetched all rows unconditionally and left all filtering to the handler. This PR pushes filtering down to SQL by introducing a sharedMCPSessionsFilterParamsstruct and a reusableapplyMCPSessionFiltershelper, and adds server-side pagination to the merged sessions list endpoint.Changes
ListAll*store methods toList*and added aMCPSessionsFilterParamsparameter to each. Filters for status, auth mode, MCP client ID, and a case-insensitive search string (matched against MCP client name/ID, user ID, session ID, and virtual key name/ID via LEFT JOIN) are now applied in SQL rather than in the handler.MCPSessionsFilterParamsinstore.gowith documentation explaining why pagination is intentionally omitted from the struct (cross-table de-dup must happen before slicing).applyMCPSessionFiltersinrdb.goas a shared helper that appends the WHERE clauses and search JOINs to a GORM query, parameterised by amcpSessionFilterTabledescriptor that carries the table name and the auth-mode column name (which differs between credential tables and flow tables).ORDER BYandWHEREcolumn references with their table names to avoid ambiguity after the search JOINs are added.listhandler inmcp_sessions.goto parse query-string filters (q,status,auth_mode,mcp_client_id,kind,limit,offset) via a newparseMCPSessionsListQueryfunction, skip store calls that cannot contribute rows given the filter shape, perform a stable cross-table sort bycreated_at DESCafter merging, and return a paginated response withcount,total_count,limit, andoffsetfields.kindAllowed,flowsAllowed, andstatusFilterAllowsPendinghelpers onmcpSessionsListQueryto cleanly express which store calls to skip.MockConfigStoreinconfig_test.goto match the new method signatures.rdb_mcp_sessions_test.gowith integration-style tests covering no-filter listing, status/auth-mode/MCP-client-ID filtering, search by MCP client name, virtual key name, user ID, and session ID, preload population, and pending-only enforcement.mcp_sessions_test.gowith unit tests forparseMCPSessionsListQuerycovering defaults, all parameters, limit capping, invalid limit/offset values, andkindAllowedlogic.Type of change
Affected areas
How to test
go test ./framework/configstore/... ./transports/bifrost-http/handlers/... ./transports/bifrost-http/lib/...Validate filtering via the sessions endpoint:
Expected: response includes
sessions,count,total_count,limit, andoffsetfields; rows match the supplied filters;total_countreflects the full unsliced result.Breaking changes
The four
ListAll*store methods on theConfigStoreinterface have been renamed and their signatures changed to acceptMCPSessionsFilterParams. Any external implementations ofConfigStoremust be updated to match the new signatures. The sessions list API response now includes additional pagination fields (count,total_count,limit,offset) alongsidesessions.Related issues
N/A
Security considerations
Search JOINs use parameterised LIKE queries; no raw user input is interpolated into SQL. Table and column names in
applyMCPSessionFiltersare hardcoded constants, not user-supplied. DAC scope enforcement remains at theScopedDB(ctx)layer and is unaffected by this change.Checklist
docs/contributing/README.mdand followed the guidelines