removes from_memory for APIs

[akshaydeo]

Summary

Removes the from_memory query parameter shortcut from all governance API endpoints and fixes a cache-poisoning risk in the filter data handler. Previously, callers could pass ?from_memory=true to bypass the database and read directly from the in-memory governance snapshot. This path is now removed entirely — all reads go through the ConfigStore — ensuring responses always reflect persisted state. Additionally, the filter data cache is now skipped for DAC-scoped requests to prevent scoped data from being served to unscoped callers (or vice versa).

Changes

• Removed from_memory branch from getVirtualKeys, getVirtualKey, getTeams, getTeam, getCustomers, getCustomer, getBudgets, getRateLimits, getModelConfigs, getProviderGovernance, getRoutingRules, and getRoutingRule — all now unconditionally delegate to ConfigStore.
• Replaced the in-memory VK name lookup in getMCPClientsPaginated with targeted GetVirtualKey calls per unique VK ID found in the fetched assignments, removing the dependency on governanceManager.GetGovernanceData.
• Introduced shouldUseFilterDataCache to gate the shared filterdata cache on both an empty query string and the absence of a DAC query scope in the request context, preventing cross-scope cache hits.

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

go test ./transports/bifrost-http/handlers/...

• Confirm that GET /api/governance/virtual-keys?from_memory=true returns the same DB-backed response as without the parameter.
• Confirm that GET /api/logs/filterdata with a DAC-scoped request context does not return cached data from an unscoped request.
• Confirm that GET /api/governance/routing-rules/{id} returns 404 for a missing rule and the correct rule for a valid ID.

Breaking changes

• Yes
• No

The from_memory=true query parameter is no longer honored on any governance endpoint. Clients relying on this parameter will continue to receive valid responses, but data will always be sourced from the database rather than the in-memory snapshot.

Security considerations

The filter data cache fix prevents a DAC-scoped request from populating or consuming the shared all-data cache, closing a potential data leakage path where one tenant's scope constraints could be bypassed by a cache hit populated under a different (or no) scope.

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 by CodeRabbit

Release Notes

• Bug Fixes

  • Governance API endpoints now consistently use database-backed responses for all requests, ensuring data consistency
  • Filter data caching now properly respects query scoping constraints
  • Virtual key enrichment improved with batch fetching for better reliability

• Tests

  • Added test coverage for governance endpoint caching behavior and filter data caching logic

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Removes from_memory=true in-memory bypass across governance GET handlers so all reads use ConfigStore; adds query-scope-aware gating for filterdata cache; updates MCP VK enrichment to batch-fetch VK names from ConfigStore; and adds tests for routing and cache behavior.

Changes

Governance Data Source & Caching Consolidation

Layer / File(s)	Summary
Governance endpoints – remove in-memory bypass transports/bifrost-http/handlers/governance.go	Removed from_memory=true checks across governance GET handlers (virtual-keys, virtual-keys/{vk_id}, teams, teams/{team_id}, customers, customers/{customer_id}, budgets, rate-limits, model-configs, providers, routing-rules, routing-rules/{rule_id}); all reads now use ConfigStore with existing pagination/filter behavior preserved; getRoutingRule always uses configStore.GetRoutingRule.
Governance tests – validate ConfigStore usage transports/bifrost-http/handlers/governance_test.go	Mocks updated to record call counts. Added tests asserting from_memory=true routes through ConfigStore non-paginated and paginated paths with zero governance-manager calls.
Filter data cache – add query scope awareness transports/bifrost-http/handlers/logging.go, transports/bifrost-http/handlers/logging_test.go	Added shouldUseFilterDataCache(ctx, query) requiring empty/whitespace query and no active query scope to enable cache. Updated /api/logs/filterdata and /api/mcp-logs/filterdata to use it. Added three tests covering unscoped empty queries, non-empty searches, and DAC-scoped contexts.
MCP clients – refactor virtual key name resolution transports/bifrost-http/handlers/mcp.go	Changed getMCPClientsPaginated to deduplicate VK IDs on the current page and batch-fetch redacted virtual keys via ConfigStore to populate names; logs batch fetch errors and leaves names unset for missing keys.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~40 minutes

Suggested reviewers

• danpiths
• roroghost17

Poem

🐰 I bounced from cache into the store,

No more in-memory detours to explore.
Twelve handlers now read straight and sure,
Filterdata keeps scopes pure,
VK names fetched in one tidy tour. 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 77.78% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'removes from_memory for APIs' directly and concisely describes the main change: removal of the from_memory query parameter from governance API endpoints.
Description check	✅ Passed	The description comprehensively covers all required sections: summary of purpose, detailed changes, type of change, affected areas, testing steps, breaking changes, security considerations, and checklist completion.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch 05-30-removes_from_memory_for_apis

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 @coderabbitai help to get the list of available commands and usage tips.}

[akshaydeo]

• removes from_memory for APIs #3903 👈 (View in Graphite)
• dev

This stack of pull requests is managed by Graphite. Learn more about stacking.

[coderabbitai]

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 `@transports/bifrost-http/handlers/governance_test.go`:
- Around line 1334-1359: The test
TestGetVirtualKeys_FromMemoryWithLimitUsesPaginatedConfigStore should also
assert the non-paginated path wasn't invoked: after calling
h.getVirtualKeys(ctx) add a check that the mock config store's non-paginated
call counter (e.g. store.getVirtualKeysCalls) is 0 so the test fails if both
GetVirtualKeys and GetVirtualKeysPaginated are called; modify the test to
include this negative-path assertion alongside the existing checks on
manager.getGovernanceDataCalls and store.getVirtualKeysPaginatedCalls.

🪄 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: ASSERTIVE

Plan: Pro

Run ID: 11cc5f8f-ffff-4f99-a024-9168f7fe9a37

📥 Commits

Reviewing files that changed from the base of the PR and between 67e57dd and 5e36679.

📒 Files selected for processing (5)

• transports/bifrost-http/handlers/governance.go
• transports/bifrost-http/handlers/governance_test.go
• transports/bifrost-http/handlers/logging.go
• transports/bifrost-http/handlers/logging_test.go
• transports/bifrost-http/handlers/mcp.go

[greptile-apps]

Confidence Score: 5/5

Safe to merge — the changes are a targeted removal of a bypass path and a focused cache-gate fix with no modifications to write paths or error-handling contracts.

All 12 handler changes are pure deletions of the from_memory branch; the retained ConfigStore delegation paths are untouched. The shouldUseFilterDataCache guard is additive and cannot produce false positives because it only widens the no-cache condition. The MCP VK enrichment swap is logically equivalent for the happy path and degrades gracefully (logged error, empty VK names) on failure. New unit tests cover the critical invariants introduced by each of the three changes.

No files require special attention.

Important Files Changed

Filename	Overview
transports/bifrost-http/handlers/governance.go	Removes the from_memory query parameter branch from all 12 governance handlers; all reads now unconditionally delegate to ConfigStore. Clean diff with no regression in error-handling logic.
transports/bifrost-http/handlers/governance_test.go	Adds call-count instrumentation to mocks and two new tests verifying that from_memory=true no longer routes to GetGovernanceData and instead hits the appropriate ConfigStore method.
transports/bifrost-http/handlers/logging.go	Introduces shouldUseFilterDataCache to gate the shared filterdata cache on both an empty query string and the absence of a DAC query scope, preventing cross-scope cache poisoning. Applied consistently to both getAvailableFilterData and getMCPLogsFilterData.
transports/bifrost-http/handlers/logging_test.go	New file with three focused unit tests covering empty-query, whitespace-only, non-empty-query, and scoped-context inputs for shouldUseFilterDataCache; complete coverage of the logic.
transports/bifrost-http/handlers/mcp.go	Replaces in-memory VK name lookup (single governance snapshot read) with a deduplication pass over assignment VK IDs followed by a single GetRedactedVirtualKeys batch call, removing the dependency on governanceManager.GetGovernanceData.

_{Reviews (2): Last reviewed commit: "removes from_memory for APIs" | Re-trigger Greptile}

[coderabbitai]

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 `@transports/bifrost-http/handlers/governance.go`:
- Around line 3263-3271: Add a regression test that exercises the DB-backed
single-rule lookup path which calls configStore.GetRoutingRule: create a test
hitting the same handler that sets from_memory=true and asserts a successful 200
response for a found rule, and a separate subtest where the mocked configStore
returns configstore.ErrNotFound to assert the handler returns a 404 and calls
SendError accordingly; also add a test where configStore returns a non-not-found
error to assert a 500 mapping. Ensure the tests validate request parsing
(query/body flags like from_memory), verify middleware ordering if applicable
(authentication/validation run before configStore lookup), and use the handler
function in governance.go and the configStore mock to target GetRoutingRule
behavior and SendError status mapping.

🪄 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: ASSERTIVE

Plan: Pro

Run ID: cf85fede-9475-475a-af15-4a1c36439428

📥 Commits

Reviewing files that changed from the base of the PR and between 5e36679 and 2d00bb7.

📒 Files selected for processing (5)

• transports/bifrost-http/handlers/governance.go
• transports/bifrost-http/handlers/governance_test.go
• transports/bifrost-http/handlers/logging.go
• transports/bifrost-http/handlers/logging_test.go
• transports/bifrost-http/handlers/mcp.go

[akshaydeo]

Merge activity

• May 30, 12:42 PM UTC: A user started a stack merge that includes this pull request via Graphite.
• May 30, 12:42 PM UTC: @akshaydeo merged this pull request with Graphite.