fix(openai/responses): preserve OpenRouter server tools (openrouter:web_search) in filterUnsupportedTools#4532
Conversation
## Summary Fixes a bug where OTEL plugin headers were being overwritten with redacted placeholder values when saving a plugin configuration. After the multi-profile change, header values stored as plain strings inside the `profiles` array were not being restored from the database before saving, causing real credentials to be replaced with masked values like `****`. ## Changes - Extracted `restoreRedactedValue` as a standalone recursive helper, replacing the inline logic in `restoreRedactedFromExisting`. This allows the restoration logic to descend into both nested maps and slices. - Added slice traversal support (index-aligned) so that elements within arrays like the OTEL `profiles` array are individually checked and restored. - Added plain-string redaction detection so that header values stored as raw strings (rather than `EnvVar` objects) are also restored from the existing DB config when they carry a redaction artifact. Empty strings are intentionally left as-is to allow clearing a value. - Added `TestRestoreRedacted_OTELProfilesHeaders` to cover both failure modes: slice traversal and plain-string secret restoration. Also asserts that genuinely new (non-redacted) values pass through unchanged. ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [x] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./transports/bifrost-http/handlers/... ``` Verify that saving an OTEL plugin configuration with multiple profiles, after a GET that returns redacted header values, does not overwrite the stored credentials in the database. Confirm that providing a genuinely new header value still persists correctly. ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations This fix ensures that redacted credential placeholders returned to the client are never written back over real secrets stored in the database. The restoration logic only replaces values that are confirmed redaction artifacts; empty strings and non-redacted values are always passed through as-is, preserving the ability to clear a credential intentionally. ## 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
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a new ChangesOpenRouter server tools namespace whitelist
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 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" Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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 `@core/providers/openai/responses.go`:
- Around line 341-343: The current implementation only allowlists
ResponsesToolTypeOpenRouterWebSearch for OpenRouter providers (lines 341-343),
but OpenRouter supports 6 server-side tools with the openrouter: prefix. Instead
of checking for individual tool type constants that don't exist for the other 5
tools, replace the explicit tool type check with a namespace-based prefix match
that identifies any tool starting with openrouter: and adds it to
supportedTypes. This approach will capture all current and future OpenRouter
tools (web_search, web_fetch, datetime, image_generation, apply_patch, subagent)
and prevent them from being silently stripped by the whitelist validation logic.
Additionally, add test coverage for at least one non-web_search OpenRouter tool
to ensure the namespace prefix matching works correctly.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 23fd0ac7-527b-4a30-952f-041bcf73be50
📒 Files selected for processing (3)
core/providers/openai/responses.gocore/providers/openai/responses_test.gocore/schemas/responses.go
…pportedTools filterUnsupportedTools stripped tools whose type was not in the OpenAI-native whitelist, including OpenRouter server tools (the "openrouter:" namespace: web_search, web_fetch, datetime, image_generation, apply_patch, subagent). For provider=openrouter on /v1/responses this dropped the tool, so the upstream call ran with tools:[] and no server tool executed (tool_choice:"required" then caused a 400 from the upstream). Allow any "openrouter:"-prefixed tool type through the filter when provider == OpenRouter, mirroring the existing XAI x_search handling. This covers all current and future OpenRouter server tools without per-tool additions. Fixes maximhq#4530
087bf22 to
0d1397d
Compare
The merge-base changed after approval.
What
filterUnsupportedTools(core/providers/openai/responses.go) strips any tool whose type is not in the OpenAI-native whitelist. OpenRouter server tools (openrouter:web_search) are not in that whitelist, so forprovider=openrouteron/v1/responsesthe tool is dropped: the upstream OpenRouter call runs withtools: [], no web search executes, and withtool_choice: "required"the upstream returns 400.Fixes #4530.
Change
Mirror the existing xAI
x_searchhandling: whenprovider == OpenRouter, allow the OpenRouter-nativeopenrouter:web_searchserver tool through the filter. Adds theResponsesToolTypeOpenRouterWebSearchconstant.Repro (before this change)
A direct OpenRouter call runs the tool and returns
url_citationannotations; the same request routed through Bifrost drops the tool and returns no citations. Full curl repro in #4530.Test
TestToOpenAIResponsesRequest_OpenRouterServerToolPreservedassertsopenrouter:web_searchis preserved forprovider=openrouterand stripped forprovider=openai.