fix: fixes model passthrough prefix stripping in advisor tool for anthropic - #3420
Merged
akshaydeo merged 9 commits intoMay 12, 2026
Conversation
Contributor
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughPreprocesses Anthropic server tool objects in the raw-body sanitizer to rewrite provider-prefixed model strings (e.g., ChangesModel ID Namespace Stripping for Anthropic Tools
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 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 docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
Contributor
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
|
roroghost17
marked this pull request as ready for review
May 12, 2026 10:16
Contributor
Confidence Score: 5/5Safe to merge; the change is narrow, well-tested, and delegates to an already-proven helper. The fix is a small, self-contained addition inside an existing per-tool loop. It reuses ParseModelString, which already handles the known-provider / unknown-namespace distinction correctly. The three new tests exercise every branch of the new code, and the existing tests remain unaffected. No structural changes, no new error paths that could propagate unexpectedly. No files require special attention. Important Files Changed
Reviews (2): Last reviewed commit: "fix: fixes model passthrough prefix stri..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
…filter inaccessible sidebar items (#3295) ## Summary This PR improves RBAC granularity in the sidebar by introducing dedicated resource types for `APIKeys`, `Inference`, and `Metrics`, and fixes sidebar visibility logic so that items and groups are hidden when the user lacks access rather than relying on broader, less specific permissions. ## Changes - Added three new `RbacResource` enum values: `APIKeys`, `Inference`, and `Metrics` to the fallback RBAC context. - The API Keys sidebar item now gates access via the new `hasAPIKeyAccess` (`RbacResource.APIKeys`) check instead of the generic `hasSettingsAccess`. - The MCP Logs sidebar item now correctly gates access via `hasMCPGatewayAccess` instead of the unrelated `hasLogsAccess`. - Introduced an `accessibleItems` memoized computation that filters out sidebar items and entire groups whose sub-items are all inaccessible, ensuring users never see empty navigation sections. Previously, access filtering only happened during search. - Removed unused imports (`PanelLeft`, `PanelRight`, `cn`). ## Type of change - [ ] Bug fix - [x] Feature - [x] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test 1. Log in as a user with restricted RBAC permissions that exclude `APIKeys` and/or `Settings`. 2. Verify the API Keys entry under the Config section is hidden for users without `APIKeys` view permission. 3. Verify the MCP Logs entry is hidden for users without `MCPGateway` view permission. 4. Verify that sidebar groups with no accessible sub-items are hidden entirely rather than showing an empty group. 5. Verify that users with full access see no change in sidebar behavior. ```sh cd ui pnpm i || npm i pnpm build || npm run build ``` ## Screenshots/Recordings _Add before/after screenshots showing sidebar items hidden for restricted users._ ## Breaking changes - [ ] Yes - [x] No ## Related issues _Link related issues here._ ## Security considerations Access control checks for API Keys management are now scoped to a dedicated `APIKeys` RBAC resource rather than the broader `Settings` resource, reducing the risk of unintended access to key management for users who have settings visibility but should not manage API keys. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
…3296) ## Summary Improves RBAC enforcement across the configuration and providers UI by hiding action controls entirely when the user lacks the required permissions, rather than rendering them in a disabled state. ## Changes - The config layout now checks the current route to determine which RBAC resource to evaluate — `APIKeys` for `/workspace/config/api-keys` routes and `Settings` for all others, so users without API key access are not incorrectly blocked from other config pages. - The "Add Provider" dropdown is now conditionally rendered only when the user has provider create access, instead of always rendering with a disabled state. - The "Add new key" button in the model provider keys table is now hidden entirely when the user lacks update access, rather than being rendered as disabled. - The per-row actions dropdown menu (Edit/Delete) in the model provider keys table is now hidden entirely when the user has neither update nor delete access. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test 1. Log in as a user with restricted RBAC permissions (no `APIKeys` view access, no provider create/update/delete access). 2. Navigate to `/workspace/config/api-keys` — the no-permission view should be shown. 3. Navigate to another config page — it should load normally. 4. Navigate to the Providers page — the "Add Provider" dropdown should not be visible. 5. Open a provider's key table — the "Add new key" button and the per-row actions menu should not be visible. 6. Log in as a user with full access and verify all controls appear and function as expected. ```sh cd ui pnpm i || npm i pnpm build || npm run build ``` ## Screenshots/Recordings If UI changes, add before/after screenshots or short clips. ## Breaking changes - [ ] Yes - [x] No ## Related issues Link related issues and discussions. ## Security considerations These changes tighten UI-level RBAC enforcement by ensuring that action controls are not rendered at all for unauthorized users, reducing the surface area for accidental or misleading interactions. Server-side authorization remains the authoritative enforcement layer. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
…elete access (#3314) ## Summary The delete button in log tables was always rendered (just disabled) for users without delete access. This PR hides the actions column entirely when the user lacks delete permissions, and fixes the RBAC resource check for MCP logs to use the correct `MCPGateway` resource instead of `Logs`. ## Changes - The actions column in both the workspace logs and MCP logs tables is now conditionally included in the column definitions only when `hasDeleteAccess` is `true`, rather than always rendering a disabled button. - The delete button styling was updated to use more visible destructive colors (`text-destructive/60 border-destructive/60`) instead of the previous muted secondary foreground styles. - The RBAC resource used to gate delete access on the MCP logs page was corrected from `RbacResource.Logs` to `RbacResource.MCPGateway`. ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test 1. Log in as a user **without** delete access on Logs or MCPGateway resources. 2. Navigate to the workspace logs page and the MCP logs page. 3. Verify the delete button/column is not visible. 4. Log in as a user **with** delete access. 5. Verify the delete button appears and is functional. ```sh cd ui pnpm i pnpm test pnpm build ``` ## Screenshots/Recordings Before: Delete button rendered but disabled for users without access. After: Delete column is hidden entirely for users without delete access. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations The RBAC fix ensures MCP log deletion is gated on the correct `MCPGateway` resource permission, preventing users with only `Logs` delete access from incorrectly being granted delete access to MCP logs. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
…ogs route and sidebar (#3316) ## Summary Introduces a dedicated `MCPLogs` RBAC resource, decoupling MCP log access control from the `MCPGateway` resource. This allows permissions for viewing and deleting MCP logs to be managed independently from gateway-level permissions. ## Changes - Added `MCPLogs` as a new `RbacResource` enum value in the fallback RBAC context. - The MCP Logs route now checks `MCPLogs` view permission and renders a `NoPermissionView` when access is denied, rather than rendering the page unconditionally. - Delete access on the MCP Logs page now checks `RbacResource.MCPLogs` instead of `RbacResource.MCPGateway`. - The sidebar MCP Logs entry now uses `hasMCPLogsAccess` (derived from `RbacResource.MCPLogs`) to control visibility, rather than reusing `hasMCPGatewayAccess`. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test 1. Configure a role that has `MCPGateway` access but **no** `MCPLogs` access. 2. Log in as a user with that role and navigate to the MCP Logs page — the `NoPermissionView` should be displayed and the sidebar entry should be hidden. 3. Grant the role `MCPLogs` view access and confirm the page and sidebar entry become accessible. 4. Verify that delete functionality on the MCP Logs page is gated by `MCPLogs` delete permission independently of `MCPGateway` delete permission. ```sh cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` ## Screenshots/Recordings N/A ## Breaking changes - [x] Yes - [ ] No Any role configuration that previously relied on `MCPGateway` permissions to grant access to MCP Logs will need to be updated to explicitly grant `MCPLogs` permissions. ## Related issues N/A ## Security considerations Access to MCP log data (which may contain sensitive tool execution details) is now enforced by a dedicated RBAC resource, reducing the risk of unintended access through overly broad `MCPGateway` permissions. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
…rom `MCPGateway` (#3319) ## Summary Introduces a dedicated `MCPToolGroups` RBAC resource to allow fine-grained access control over the MCP Tool Groups section, independent of the broader `MCPGateway` resource. ## Changes - Added `MCPToolGroups` as a new `RbacResource` enum value in the fallback RBAC context. - Updated the MCP Tool Groups route layout to check `MCPToolGroups` view permission instead of `MCPGateway`. - Updated the sidebar so the "Tool Groups" sub-item uses `hasMCPToolGroupsAccess`, while the parent MCP nav item remains visible if the user has access to either `MCPGateway` or `MCPToolGroups`. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test 1. Configure a role that has access to `MCPGateway` but not `MCPToolGroups`. Verify the "Tool Groups" sidebar item is hidden and navigating to `/workspace/mcp-tool-groups` shows the no-permission view. 2. Configure a role with access to `MCPToolGroups` but not `MCPGateway`. Verify the "Tool Groups" sidebar item is visible and accessible, while other MCP Gateway sections remain restricted. 3. Configure a role with access to both. Verify all MCP sub-items are visible and accessible. ```sh cd ui pnpm i || npm i pnpm build || npm run build ``` ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations Access to the MCP Tool Groups page is now governed by its own RBAC resource (`MCPToolGroups`), allowing enterprise deployments to restrict tool group management independently from MCP Gateway configuration. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
## Summary Removes the upper bound limit cap of 100 on paginated team queries, allowing callers to request more than 100 teams per page. ## Changes - Removed the `limit > 100` guard in `GetTeamsPaginated` that was silently capping the page size to 100. This allows consumers to specify larger page sizes when needed. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./... ``` Verify that a call to `GetTeamsPaginated` with a `limit` greater than 100 returns the expected number of results rather than being silently truncated to 100. ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations Removing the page size cap could allow large queries that put additional load on the database. Callers should ensure reasonable limits are enforced at the API layer if unbounded queries are a concern. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
#3389) ## Summary Briefly explain the purpose of this PR and the problem it solves. ## Changes - What was changed and why - Any notable design decisions or trade-offs ## Type of change - [ ] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test Describe the steps to validate this change. Include commands and expected outcomes. ```sh # Core/Transports go version go test ./... # UI cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` If adding new configs or environment variables, document them here. ## Screenshots/Recordings If UI changes, add before/after screenshots or short clips. ## Breaking changes - [ ] Yes - [ ] No If yes, describe impact and migration instructions. ## Related issues Link related issues and discussions. Example: Closes #123 ## Security considerations Note any security implications (auth, secrets, PII, sandboxing, etc.). ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
## Summary Bumps several Go dependencies to their latest patch versions across all modules in the repository. ## Changes - `github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream` upgraded from `v1.7.8` → `v1.7.10` - `github.com/aws/smithy-go` upgraded from `v1.24.2` → `v1.25.1` - `github.com/jackc/pgx/v5` upgraded from `v5.9.1` → `v5.9.2` ## Type of change - [ ] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [x] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [x] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./... ``` ## Breaking changes - [ ] Yes - [x] No ## Security considerations No security implications. These are routine patch-level dependency upgrades with no API surface changes. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] 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
akshaydeo
dismissed
coderabbitai[bot]’s stale review
May 12, 2026 11:11
The merge-base changed after approval.
roroghost17
force-pushed
the
05-12-fix_fixes_model_passthrough_prefix_stripping_in_advisor_tool_for_anthropic
branch
from
May 12, 2026 13:09
3a2eccb to
1cd17b8
Compare
akshaydeo
dismissed
coderabbitai[bot]’s stale review
May 12, 2026 15:23
The base branch was changed.
akshaydeo
deleted the
05-12-fix_fixes_model_passthrough_prefix_stripping_in_advisor_tool_for_anthropic
branch
May 12, 2026 15:24
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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
When clients (e.g. Claude Code's
/advisorintegration) read Bifrost's model catalog, they embed provider-prefixed model IDs (e.g.anthropic/claude-opus-4-7) inside server tool definitions likeadvisor_20260301. Anthropic's upstream API rejects these prefixed IDs, returning an error ontools.N.model. This PR strips the known Bifrost provider prefix from a tool'smodelfield before the request is forwarded, while leaving bare IDs and unknown namespaces (e.g.meta-llama/Llama-3.1-8B) untouched.Resolves issue: #3383
Changes
StripUnsupportedFieldsFromRawBody, added logic to inspect each tool'smodelfield. IfParseModelStringidentifies a known Bifrost provider prefix, the prefix is stripped and only the bare model ID is written back into the request body.Type of change
Affected areas
How to test
go test ./core/providers/anthropic/... -run TestStripUnsupportedFieldsFromRawBodyExpected: all three new subtests (
advisor_tool_model_prefix_stripped,advisor_tool_bare_model_passes_through,advisor_tool_unknown_prefix_passes_through) pass alongside existing tests.Screenshots/Recordings
N/A
Breaking changes
Related issues
N/A
Security considerations
No auth, secrets, or PII involved. The change only rewrites model ID strings in outbound request bodies to conform to Anthropic's API requirements.
Checklist
docs/contributing/README.mdand followed the guidelines