Repository navigation
TXE-5174: Merge upstream dev ec081ef5b (v2.3.0 slice 1) as transport v2.3.0-sync.1 - #28
Merged
Merged
Conversation
…T omits key_ids (maximhq#7362) The existing-config branch of updateVirtualKey derived allowAllKeys and keys solely from the request's key_ids, so a client doing GET -> edit -> PUT (the response carries allow_all_keys/keys but never key_ids) had its associations wiped: AllowAllKeys flipped to false and every key detached. Subsequent inference through the VK failed with 'no keys found'. Treat an absent key_ids as 'unchanged' (schemas.WhiteList is a slice, so nil after unmarshal is distinguishable from an explicit empty list), and preload Keys on the existing-config rows since the store update replaces the association with whatever the row carries. An explicit [] still clears; an explicit list still replaces. Fixes maximhq#7347
…tead of tearing it down (maximhq#6921) ## Summary When a make-before-break re-dial fails for a generic reason (network blip, credential update that hasn't propagated yet, the periodic checker's own recovery attempt), the previously working connection was being torn down unnecessarily. This turned every failed re-dial into a real outage even when the existing connection was still healthy. This PR introduces a classification split in `failConnectAttempt`: a survivable failure keeps the live connection installed on the entry, while a dead-credential failure (where the upstream has confirmed the credential is worthless) still tears it down. ## Changes - `failConnectAttempt` now distinguishes two teardown shapes based on whether `oldConn != nil` and the failure is not a dead-credential classification: - **Survivable failure (make-before-break with a live connection):** The old connection and its cancel func are restored on the entry, `ConnGeneration` is left unchanged (so late writers bound to this generation remain valid), and only the half-built new handles are closed. State moves to `Unstable` to record the failure, but tool calls keep flowing over the surviving connection. - **Everything else (first dial, close-first path, or dead credential):** The entry is fully detached — connection set to nil, connection info reset, generation bumped — and both new and old handles are closed. State becomes `Unstable` or `NeedsReauth` accordingly. - The `failConnectAttempt` doc comment is rewritten to describe both shapes explicitly, including the load-bearing ordering rationale that applies only to the detaching shape. - `TestConnectToMCPClient_MakeBeforeBreak_DialFailure_ResetsStateAndClosesOldConn` is updated to `TestConnectToMCPClient_MakeBeforeBreak_DialFailure_PostFailureState` and extended with a `keepsOldConn` field per test case, asserting the correct connection pointer, generation, and reachability for each classification. - A new test `TestConnectToMCPClient_MakeBeforeBreak_DialFailure_KeepsLiveConnectionServing` verifies end-to-end that tool calls continue succeeding over the surviving connection after a failed re-dial. - A new test `TestPerformCheck_LiveConn_FailedCheck_TriggersReconnect` pins the `ClientConnectionChecker` live-connection branch: a failed probe of a dead session must mark the client `Unstable` and start a background reconnect that installs a fresh connection, rather than re-probing the same dead session indefinitely. ## Type of change - [x] Bug fix ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) ## How to test ```sh go test ./core/mcp/... -run TestConnectToMCPClient_MakeBeforeBreak_DialFailure go test ./core/mcp/... -run TestPerformCheck_LiveConn_FailedCheck_TriggersReconnect go test ./core/mcp/... ``` After a failed re-dial on a client with an existing live connection, the client should remain in `Unstable` state with its original connection still installed and serving tool calls. Only a `NeedsReauth` failure should detach and close the connection. ## Breaking changes - [ ] Yes - [x] No ## Security considerations None. The change affects connection lifecycle management only; no auth flows, secrets handling, or user data are modified. ## 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
…re-probing the same dead session (maximhq#6922) ## Summary When a live connection check fails, the dead connection remains installed on the client entry. Without an explicit reconnect trigger at that point, every subsequent checker tick re-probes the same dead session indefinitely, and the `Conn == nil` branch that would normally initiate a reconnect is never reached while a connection is installed. ## Changes - Added `reconnectAfterFailedCheck` which fires a background `ReconnectClient` call immediately after a failed live connection check, so the dead connection is replaced rather than re-probed on every tick. - The method includes a staleness guard: if the connection generation has advanced (a reconnect already replaced the dead session), or the client is in `Disabled` or `NeedsReauth` state, the reconnect is skipped. `Disabled` and `NeedsReauth` are checked independently of the generation because neither `DisableClient` nor `CloseAndMarkNeedsReauth` bumps `ConnGeneration` when clearing the connection. - Concurrent reconnect attempts (e.g. from the reactive per-request path) are safe to ignore since `ReconnectClient` deduplicates attempts per client through its own exclusive-op guard. - Added tests covering the staleness guard cases: a check that spanned a reconnect, a client disabled mid-check, a client in `NeedsReauth`, and a current-generation failure on a live client that should redial. ## Type of change - [x] Bug fix ## Affected areas - [x] Core (Go) ## How to test ```sh go test ./core/mcp/... ``` The new test `TestReconnectAfterFailedCheck_StaleOrAuthoritativeState_DoesNotReconnect` validates the guard logic across all four cases. The existing `TestPerformCheck_LiveConn_FailedCheck_TriggersReconnect` confirms the dead connection is replaced rather than re-probed. ## Breaking changes - [x] No ## Security considerations None. This change only affects reconnect scheduling logic for MCP client connections and does not touch auth, secrets, or PII handling. ## 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
…nal`, renaming `attemptAuthFailureRecovery` to `attemptCallFailureRecovery` (maximhq#6923) ## Summary When an MCP server abandons a session, tool calls over that connection fail with errors that are neither auth-shaped nor matched by `ToolCallRetryConfig`, so they were never retried or recovered. Every subsequent call failed the same way until the periodic connection checker happened to reconnect — up to a full check interval away. This PR extends the existing auth-failure recovery path to also handle dead sessions, reconnecting immediately and salvaging the caller's request where safe to do so. ## Changes - Added `isDeadSessionErrorText` in `utils.go` with a narrow set of substrings that identify a dead/abandoned MCP session (e.g. `"session terminated"`, `"need to re-initialize"`, `"session not found"`). Generic transport failures are intentionally excluded to avoid false positives. - Extended the failure-detection branch in `executeToolInternal` to trigger on dead-session errors in addition to auth failures, since the remedy (reconnect + conditional retry) is identical for both. - Renamed `attemptAuthFailureRecovery` → `attemptCallFailureRecovery` and `recoverSharedConnection`'s doc references updated accordingly, reflecting that the function now handles both failure classes. - For shared connections, the background reconnect is always triggered even when the retry is suppressed (e.g. for unannotated/destructive tools), so the connection heals behind the failed call rather than waiting on the periodic checker. - Added `toolcall_recovery_test.go` with two tests: - `TestExecuteTool_DeadSession_SalvagesTheCallForARetryableTool`: verifies that a read-only (idempotent) tool call over a dead session is transparently reconnected and retried, with the caller seeing success and a new connection generation confirmed. - `TestExecuteTool_DeadSession_HealsEvenWhenTheRetryIsSuppressed`: verifies that an unannotated (assumed destructive) tool call is not replayed, but the connection is still repaired so the next call succeeds. ## Type of change - [x] Bug fix ## Affected areas - [x] Core (Go) ## How to test ```sh go test ./core/mcp/... -run TestExecuteTool_DeadSession ``` Both tests spin up a real `httptest.Server` backed by an MCP server, expire the session mid-test, and assert on recovery behavior and connection generation changes. ## Breaking changes - [x] No ## Related issues ## Security considerations The dead-session substring list is intentionally narrow to avoid triggering reconnects on generic transport errors, which could cause unnecessary session churn. The retry gate (`attemptCallFailureRecovery`) already requires tool annotations indicating read-only/idempotent behavior before replaying any call, preventing unintended duplicate side effects. ## 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
…s permanent in `ProbeRetryConfig` (maximhq#6924) ## Summary The periodic connection checker's probe retries (`ProbeRetryConfig`) were using the shared `isTransientError` classifier, which treats timeouts as permanent failures. This is appropriate for initial dial attempts, but incorrect for heartbeat pings over an already-established connection — a single slow response from a busy upstream would exhaust none of the retry budget and immediately mark the client as Unstable, potentially churning a healthy session over a transient hiccup. ## Changes - Introduced `isTransientProbeError`, a dedicated retry classifier for `ProbeRetryConfig` that treats timeouts as retryable (unlike the shared classifier) while still treating dead sessions (404/422) and auth rejections (401/403) as permanent failures — since retrying those over the same connection cannot succeed and only delays the reconnect. - Auth rejections are evaluated before the timeout rule to prevent error messages like `"403 forbidden: timeout"` from incorrectly falling into the retryable timeout branch. - Wired `isTransientProbeError` into `ProbeRetryConfig` via the `IsRetryable` field. - Added `TestProbeRetryConfig_RetriesTimeoutsButNotPermanentFailures` to pin the classification behavior: timeouts are retried up to `MaxRetries + 1` attempts, while dead sessions and auth failures fail on the first attempt. ## Type of change - [x] Bug fix ## Affected areas - [x] Core (Go) ## How to test ```sh go test ./core/mcp/... ``` The new test `TestProbeRetryConfig_RetriesTimeoutsButNotPermanentFailures` validates that: - Timeout errors (`context deadline exceeded`, `i/o timeout`) are retried the full number of configured attempts. - Dead session errors (404, 422) and auth rejections (401, 403) fail immediately on the first attempt, even when the error text also contains the word "timeout". ## Breaking changes - [x] No ## Security considerations Auth rejection errors (401, 403) are explicitly classified as permanent and non-retryable, ensuring credentials are not repeatedly sent against a rejecting endpoint. ## 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
…retrying and rename test to match (maximhq#6925) ## Summary Clarifies the actual retry behavior when `EnableClient` fails to connect: a checker is registered but immediately stops itself on its first tick because `performCheck` exits early for `Disabled` clients rather than attempting a redial. Recovery requires the admin to explicitly call enable again, which the `Disabled` parking state is designed to keep possible. ## Changes - Updated the comment in `EnableClient` to accurately describe that the registered checker does **not** automatically retry the dial for a `Disabled` client — `performCheck` stops it on first tick. The checker is retained only because it serves the `NeedsReauth` case, where it keeps the timer alive at a relaxed interval to eventually pick up a stalled reauthorization. - Renamed `TestEnableClient_ConnectFailure_ParksDisabledAndKeepsRetrying` to `TestEnableClient_ConnectFailure_ParksDisabledAndStaysEnableable` to remove the misleading implication that the checker retries the dial automatically. - Updated the test's assertion message and doc comment to explicitly note that the checker stops itself on first tick for a `Disabled` client rather than dialling, and that recovery is admin-initiated via a re-enable. ## Type of change - [ ] Bug fix - [ ] Feature - [ ] Refactor - [x] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./core/mcp/... ``` The renamed test `TestEnableClient_ConnectFailure_ParksDisabledAndStaysEnableable` should pass and continue to assert that the checker is registered, the `ExecutionConfig` remains un-disabled, and the state is parked at `Disabled` rather than `Unstable`. ## Breaking changes - [x] No ## Security considerations None. This is a documentation and test naming correction with no behavioral changes. ## Checklist - [ ] 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) - [ ] I verified the CI pipeline passes locally if applicable
… connection checker (maximhq#6926) ## Summary When an MCP client is configured with an OAuth or per-user-headers/token-exchange auth type but has never completed its one-time admin authorization flow, it should remain in `pending_verification` state so the UI can surface the "Verify" CTA. A race condition existed where `EnableClient` would bypass this state entirely — either marking the client `Healthy` outright (per-call branch) or attempting to dial with a credential that doesn't exist yet (sticky branch) — causing the actionable "an admin must authorize this" state to be silently lost. ## Changes - Added `awaitsAdminVerification` predicate that identifies clients carrying an unresolved `PendingOAuthConfig` (OAuth/per-user-OAuth) or an empty `DiscoveredTools` (per-user-headers/token-exchange), mirroring the branching logic already in `AddClient`. - `EnableClient` now checks this predicate before taking any connection path. If the client still awaits admin authorization, it is parked directly into `pending_verification` with connection info populated, and no checker is started — matching exactly what `AddClient` would have done. - `performCheck` now treats `pending_verification` the same as `needsReauth`: it stays quiet and returns the steady interval without attempting any discovery, since there is no credential to check with and a failed check would only replace an actionable state with a generic `Unstable`. - `setState` now guards `pending_verification` as an authoritative terminal state alongside `Disabled` and `NeedsReauth`, preventing any in-flight check result from overwriting it after a client has been parked there. - Added `pendingverification_test.go` covering all three invariants: `EnableClient` preserving `pending_verification`, `performCheck` staying quiet, and `setState` refusing to overwrite the state. ## Type of change - [x] Bug fix ## Affected areas - [x] Core (Go) ## How to test ```sh go test ./core/mcp/... -run TestEnableClient_StillPendingVerification_KeepsThatState go test ./core/mcp/... -run TestPerformCheck_PendingVerification_StaysQuiet go test ./core/mcp/... -run TestSetState_PendingVerification_NotOverwritten go test ./core/mcp/... ``` Expected: all tests pass. Specifically, an OAuth client that was disabled before its admin authorization flow completed should transition to `pending_verification` on enable, not `Healthy` or `Unstable`, and the Verify CTA should remain visible in the UI. ## Breaking changes - [x] No ## Security considerations This fix ensures that clients requiring admin OAuth authorization cannot be silently promoted to an active/healthy state without the authorization flow completing. This prevents a client from being used with a credential that does not yet exist. ## 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
…aximhq#6930) ## Summary This PR adds test coverage for a previously untested failure mode: when an MCP client's underlying transport dies (either a STDIO subprocess being killed or an SSE stream being dropped), the periodic connection checker is the only mechanism capable of repairing it. The dead connection is never detached from the client entry, so the checker's live-connection branch must detect the stale state and reconnect. These tests verify that the checker correctly respawns the STDIO subprocess and re-establishes the SSE stream. ## Changes - Added `TestSTDIOSubprocessKilled_CheckerRespawnsIt` in the integration test package: launches a real STDIO server via a shell wrapper that records its PID, kills the process directly with `SIGKILL`, then runs a fast-cadence checker and asserts that a new process is spawned and the client returns to a healthy state. - Added `TestSSEStreamDropped_CheckerReconnects` in the `mcp` package: spins up an in-process SSE server, drops all established streams via `CloseClientConnections`, and asserts that a single `performCheck` call re-establishes the stream, increments `ConnGeneration`, and repopulates the tool map. Both tests specifically target the live-connection branch of the checker, which previously never triggered a reconnect for these transport-level failures. ## Type of change - [x] Bug fix ## Affected areas - [x] Core (Go) ## How to test ```sh # Build the STDIO test server binary first make setup-mcp-tests # Run the new integration test go test ./core/internal/mcptests/ -run TestSTDIOSubprocessKilled_CheckerRespawnsIt -v -timeout 120s # Run the SSE unit test go test ./core/mcp/ -run TestSSEStreamDropped_CheckerReconnects -v -timeout 60s ``` The STDIO test will be skipped automatically if the `go-test-server` binary has not been built. ## Screenshots/Recordings N/A ## Breaking changes - [x] No ## Related issues N/A ## Security considerations None. Tests use local processes and in-process HTTP servers with no external network access or secrets. ## 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
…tomatic reconnect recovery, and simplified config knob (maximhq#6931) ## Summary Updates the MCP connection health monitoring documentation to accurately reflect how the health check system actually works, including automatic reconnection behavior, adaptive check cadence, and how `unstable` state is handled during tool calls. ## Changes - Corrected the health check description: a check now performs both a ping and a `tools/list` over the same connection, covering liveness and tool-list refresh in one pass - Replaced the fixed 10-second interval model with the accurate adaptive cadence: healthy clients check on the `tool_sync_interval` (default 10 minutes), while `unstable` clients check every 10 seconds for fast recovery detection - Replaced the "5 consecutive failures" threshold model with the accurate single-failed-check threshold, clarifying that retry/backoff logic lives inside each check rather than across checks - Documented automatic reconnection: a failed check reconnects the client in the background (make-before-break), replacing expired sessions, dropped SSE streams, or exited `stdio` processes without manual intervention - Documented that tool call failures due to rejected credentials or gone sessions also trigger the same reconnect path, typically retrying the caller's request on the healed connection - Clarified that `unstable` does not suppress tool calls — it reflects Bifrost's own probe results only - Updated the configuration section to reflect that `health_monitor_config` with `check_interval`, `check_timeout`, and `max_consecutive_failures` is replaced by `tool_sync_interval` as the single configurable knob, settable globally or per client - Added the automatic reconnect transition to the connection state flowchart (`unstable` → `connected` via automatic reconnect) - Expanded the `unstable` state description in `connections.mdx` to explain both healing paths: failed periodic checks and failed tool calls ## Type of change - [ ] Bug fix - [ ] Feature - [ ] Refactor - [x] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [x] Docs ## How to test Review the rendered documentation for `docs/mcp/connections.mdx` and `docs/mcp/overview.mdx` to confirm: - The state flowchart includes the `unstable` → `connected` automatic reconnect transition - The health monitoring section accurately describes adaptive cadence, single-check failure threshold, and automatic reconnection - The configuration example uses `tool_sync_interval` and no longer references the removed `health_monitor_config` fields ## Breaking changes - [ ] Yes - [x] No ## Security considerations None. Documentation-only change. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [x] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
…ss all client types (maximhq#6927) ## Summary Operators who change a tool on an upstream MCP server previously had no on-demand way to make Bifrost pick it up — they had to wait for the periodic connection checker's tool-sync interval (10 minutes by default) or restart the gateway. For per-call clients (any per-user auth type, or any shared HTTP client without session stickiness), the situation was worse: `ReconnectClient` rejects those outright since they hold no persistent connection, leaving them with no refresh path at all until the next checker tick. This PR adds `POST /api/mcp/client/{id}/refresh-tools`, which re-discovers a client's tools from its upstream server immediately and persists the result through the same tools-change callback every other discovery path uses. ## Changes - **`RefreshClientTools` on `MCPManager`**: three discovery shapes mirroring the connection checker's own branches — tools/list over a live connection for sticky clients, an ephemeral connect-discover-close cycle for per-call clients, and a full reconnect for sticky clients whose connection is currently down. Returns the number of tools the client is serving after the refresh. - **`writeBackDiscoveredTools` extracted to `MCPManager`**: the generation-guarded write-back that was previously private to `ClientConnectionChecker` is now a method on the manager, shared by both the periodic checker and the new on-demand path. The checker's own `writeBackTools` method is removed. - **State guard**: disabled, `needs_reauth`, and `pending_verification` clients are rejected with `ErrMCPRefreshNotApplicable` (mapped to HTTP 400). The `pending_verification` guard is particularly important for `token_exchange` clients, where a refresh would silently succeed and bypass the one-time admin verification flow by persisting tools before the admin has confirmed them. - **`ErrMCPClientNotFound` and `ErrMCPRefreshNotApplicable`** added to `schemas/mcp.go` so HTTP handlers can map them to 404 and 400 respectively without string matching. - **HTTP handler and routing**: `POST /api/mcp/client/{id}/refresh-tools` registered in the MCP handler, with appropriate error mapping and a `tool_count` field in the success response. - **OpenAPI spec** updated with the new endpoint, including parameter, response schema, and error responses. - **Tests**: `refreshtools_test.go` covers per-call rediscovery, sticky live-connection relisting, tools-change callback firing (and non-firing on unchanged sets), unknown client errors, awaiting-admin-verification refusal across all four applicable auth types, and disabled/needs-reauth refusal. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [x] Docs ## How to test ```sh go test ./core/mcp/... ./transports/bifrost-http/... ``` To exercise the endpoint end-to-end: 1. Start Bifrost with an MCP client configured (HTTP, any auth type). 2. Add a new tool to the upstream MCP server. 3. `POST /api/mcp/client/{id}/refresh-tools` — the response should include the updated `tool_count` and the new tool should appear in subsequent tool listings without waiting for the sync interval. 4. Repeat with a per-call client (no `needs_session_stickiness`) to confirm it works where `reconnect` would have returned an error. ## Breaking changes - [ ] Yes - [x] No ## Related issues Closes maximhq#6885 ## Security considerations The endpoint is gated behind `ManagementBearerAuth` (same as reconnect and all other management operations). No credentials or secrets are exposed; the handler only triggers a tools/list against an already-configured upstream connection. ## 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
…ol re-discovery (maximhq#6928) ## Summary Adds a "Refresh tools" action to the MCP client actions menu, allowing operators to immediately re-discover tools from an upstream MCP server without waiting for the automatic tool sync interval (10 minutes by default). This is distinct from "Reconnect," which recycles the connection and does not apply to per-call clients — "Refresh tools" works for every client type. ## Changes - Added a `refreshMCPClientTools` RTK Query mutation that calls `POST /mcp/client/{id}/refresh-tools` and returns the updated tool count. - Added a "Refresh tools" dropdown menu item (using the `ListRestart` icon) to `MCPClientActionsMenu`, visible to users with update access. The item is disabled when the client is disabled, pending verification, or needs reauthorization. - Added `handleRefreshTools` handler that tracks in-progress refresh state per client, shows a toast with the resulting tool count on success, and triggers a refetch of the client list. - The actions menu button and loading spinner now account for the refreshing-tools state alongside reconnecting, reauthorizing, and verifying exchange states. ## 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. Navigate to the MCP Registry in the workspace UI. 2. Open the actions menu (`...`) for any connected MCP client. 3. Click **Refresh tools**. 4. Verify a success toast appears showing the client name and current tool count (e.g., "Client my-server is now serving 5 tools."). 5. Verify the action is disabled for clients that are disabled, pending verification, or need reauthorization. 6. Verify the spinner appears on the actions button while the refresh is in progress. ```sh cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` ## Screenshots/Recordings _Add before/after screenshots of the actions menu showing the new "Refresh tools" item._ ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations The "Refresh tools" action is gated behind `hasUpdateAccess`, consistent with other mutating actions in the MCP client actions menu. ## 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
…ross MCP connection docs (maximhq#6929) ## Summary Documents the `POST /api/mcp/client/{id}/refresh-tools` endpoint, which allows on-demand re-discovery of a client's tool list without waiting for the next periodic sync (10 minutes by default). Unlike reconnect, this endpoint applies to every client type — sticky, per-call, and per-user — making it the only universal on-demand refresh path available. ## Changes - Added a `refresh-tools` curl example alongside the existing `reconnect` example in the connecting-to-servers reference, with a note explaining how it differs from reconnect. - Added a new **Manual Tool Refresh** section under connecting-to-servers with Gateway API, Web UI, and Go SDK tabs showing usage and the expected JSON response. - Updated the endpoint comparison table in connections.mdx to include `refresh-tools` across all client types, and added a paragraph clarifying why it is the one endpoint that applies universally — it re-discovers the tool list rather than recovering a connection or credential. - Added a paragraph in overview.mdx noting that the periodic sync is not the only refresh path and pointing readers to the new Manual Tool Refresh section. ## Type of change - [ ] Bug fix - [ ] Feature - [ ] Refactor - [x] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [x] Docs ## How to test Review the rendered docs to confirm: - The `refresh-tools` curl snippet appears correctly in the API reference alongside `reconnect`. - The **Manual Tool Refresh** section renders with all three tabs (Gateway API, Web UI, Go SDK) and the correct JSON response block. - The endpoint comparison table in connections.mdx includes the `refresh-tools` row with the correct ✅/❌ indicators and descriptions. - Internal anchor links (`#manual-tool-refresh`, `./connecting-to-servers#manual-tool-refresh`, `./connecting-to-servers#global-tool-manager-settings`) resolve correctly. ## Breaking changes - [x] No ## Security considerations None. This is documentation only and does not introduce new auth flows or expose sensitive data. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [x] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
…d" config status is not mistaken for a completed consent (maximhq#7041) ## Summary When an MCP client reauthorizes OAuth consent, its `oauth_configs` row has been `"authorized"` since the original bootstrap and never regresses. Polling the bare config status endpoint during a reauthorize flow therefore reads `"authorized"` on the very first tick, before the admin has signed in upstream, causing the dashboard to close the consent popup prematurely. This PR fixes that by threading a `flow_id` through the reauthorize response and status polling path so the server resolves status from the live flow row rather than the stale config row. ## Changes - **`POST /api/mcp/client/{id}/reauthorize`** now includes `flow_id` and a corrected `status_url` (with `?flow_id=` appended) in its response, so callers know which flow row to poll against. - **`GET /api/oauth/config/{id}/status`** accepts an optional `flow_id` query parameter. When present, it resolves the returned `status` from the flow row via a new `resolveOAuthFlowPollStatus` function rather than from the config row. The response echoes `flow_id` and includes a `flow_status` field with the raw flow row state. A `flow_id` that belongs to a different config returns 404. - **`resolveOAuthFlowPollStatus`** encodes the resolution logic: `pending`/`claiming` flow rows return `"pending"`, an expired-deadline pending row returns `"expired"`, a `failed` row returns `"failed"`, and a gone row falls back to the config status (with a `"pending"` config treated as `"expired"` since the bootstrap flow was swept without completing). - A new `"expired"` status value is added to the `OAuthConfigStatus` schema and the `OAuthStatusResponse` type. - The UI `OAuth2Authorizer` component now accepts `flowId` and `expiresAt` props, passes `flow_id` on every status poll, and stops polling with a timeout error once the flow deadline passes rather than waiting on a row the server may have already swept. - The `getOAuthConfigStatus` RTK Query endpoint is updated to accept either a plain config ID string or an object with `oauthConfigId` and optional `flowId`. - The `onConflict` handler in `mcpClientsTable` is corrected: a 409 during reauthorize means the consent did not actually complete, so it now shows an error toast instead of a false success. - A table-driven unit test (`mcp_oauth_flow_poll_status_test.go`) covers all branches of `resolveOAuthFlowPollStatus`. - OpenAPI docs are updated to document the new `flow_id` query parameter, the `flow_id`/`flow_status` response fields, the `expired` enum value, and the updated 404 condition. ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (React) - [x] Docs ## How to test 1. Configure an MCP client with admin-mode OAuth and complete the initial authorization so the config status is `"authorized"`. 2. Trigger a reauthorize flow. Confirm the response includes `flow_id` and that `status_url` contains `?flow_id=`. 3. Poll `GET /api/oauth/config/{id}/status?flow_id={flow_id}` before completing the upstream consent. Confirm the response returns `status: "pending"` rather than `"authorized"`. 4. Complete the upstream consent. Confirm the next poll returns `status: "authorized"`. 5. Trigger a reauthorize flow and let it expire without completing. Confirm the poll returns `status: "expired"`. 6. Confirm that polling without `flow_id` still returns the config's own status, unaffected. ```sh # Core/Transports go test ./transports/bifrost-http/handlers/... # UI cd ui pnpm i || npm i pnpm build || npm run build ``` ## Breaking changes - [x] No The `flow_id` query parameter is optional and the endpoint is fully backward-compatible. The new `"expired"` status value is additive. ## Security considerations A `flow_id` supplied by the caller is validated against the config ID in the URL before any data is returned. A flow belonging to a different config returns 404, preventing a poll on config A from reading the state of config B's flow. ## Checklist - [ ] 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) - [ ] I verified the CI pipeline passes locally if applicable
…nd clarify `allow_by_default` scope in v2.1.0 changelog (maximhq#7050) ## Summary Updates the v2.1.0 enterprise changelog to document a new breaking change and clarify existing behavior around `allow_by_default` for keyless users. ## Changes - Adds a new breaking change entry: **Authenticated Users Without an Access Profile Are Refused**. Users authenticating via SSO bearer token, dashboard session, or Bifrost-issued user-mode MCP token who hold no access profiles are now refused with `401 access_not_found` on `/mcp`, `/mcp/<slug>`, and all inference routes. Previously, user-mode MCP tokens for such users fell through to the unfiltered gateway, exposing all tools of every connected MCP client, and inference requests passed with only user-level limits. Requests presenting a virtual key are unaffected. - Clarifies the `allow_by_default` entry to reflect that default-allowed MCP clients are granted to every access profile holder unconditionally, whether or not the request presents a key (previously stated only for keyless users). ## Type of change - [ ] Bug fix - [ ] Feature - [ ] Refactor - [x] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [x] Docs ## How to test Review the rendered changelog at `docs/changelogs/ent-v2.1.0.mdx` and confirm: - The new breaking change entry for unauthenticated users without access profiles appears correctly and links to the access profiles docs. - The `allow_by_default` entry reflects the updated wording. ## Breaking changes - [x] Yes - [ ] No Users authenticating without a virtual key (SSO, dashboard session, or Bifrost-issued user-mode MCP token) who have no access profiles assigned will now receive `401 access_not_found` instead of falling through to the unfiltered gateway. Every SSO or SCIM user must be given an access profile granting the providers and MCP clients they need, either via the default profile on their RBAC role or through a SCIM attribute mapping rule. ## Related issues See [Access Profiles Docs](https://docs.getbifrost.ai/enterprise/access-profiles) ## Security considerations The core behavioral change being documented closes a security gap where user-mode MCP tokens for users without access profiles could reach the unfiltered gateway, exposing all tools of every connected MCP client. This changelog entry ensures operators are aware of the change and the required migration steps. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [x] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
…n is rejected during delegated exchange (maximhq#7051) ## Summary When a token-exchange MCP server rejects a request because no subject token was available, the error message now distinguishes between two distinct situations: the caller never sent an identity-provider token, versus the caller sent one that failed validation. Previously, both cases produced the same generic "send a token" message, which is unhelpful to a caller who already sent a token and had it rejected. Additionally, the tool-result message for auth-required errors is refactored so that delegated exchange errors (which have no interactive URL to open) pass through the resolver's own message directly, rather than prompting the caller to open an empty URL. ## Changes - Added `MCPInboundBearerOmittedReason` type and `MCPInboundBearerRejected` constant to `core/schemas/mcp.go` to represent why an inbound bearer token was not forwarded to token exchange. - Added `BifrostContextKeyMCPInboundBearerOmitted` context key to `core/schemas/bifrost.go` so the upstream auth layer can record a token rejection for downstream use. - Replaced the inline error message string in `token_exchange.go` with `subjectTokenMissingMessage`, which reads the context key and returns a rejection-specific message when the auth layer recorded a failed token, or a "please send one" message when no token was presented at all. - Extracted `mcpAuthRequiredToolResult` in `mcpserver.go` to handle all auth-required tool result formatting. Exchange errors with no URL now pass through the resolver's message directly; interactive kinds with no URL also fall back to their message rather than prompting the caller to open nothing. - Added tests covering the new message branching in `token_exchange_test.go` and the full set of tool-result formatting cases in the new `mcpserver_toolresult_test.go`. ## Type of change - [x] Bug fix - [ ] 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 ./core/mcp/credstore/... ./transports/bifrost-http/handlers/... ``` - A request that carries no identity-provider token receives a message instructing it to send one as the Authorization bearer. - A request whose identity-provider token was rejected by the auth layer receives a message stating the token was rejected and advising the caller to re-authenticate with their identity provider. - A tool result for a delegated-exchange auth-required error carries the resolver's message and does not contain "Open this URL". - A tool result for an OAuth or headers auth-required error with a URL still contains the URL and the appropriate action phrase. - A tool result for an interactive kind with no URL falls back to the error's message field. ## Breaking changes - [ ] Yes - [x] No ## Security considerations `BifrostContextKeyMCPInboundBearerOmitted` records only the reason string (`"rejected"`), not the token itself. The existing `BifrostContextKeyMCPInboundBearer` key, which holds the live credential, is unchanged and must never be logged. ## 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
…register` to recover from evicted DCR registrations (maximhq#7228) ## Summary Fixes a recovery dead-end (issue maximhq#7191) where an MCP client using dynamic client registration (RFC 7591) becomes permanently broken after the upstream authorization server restarts and forgets its client registry. Once the stored `client_id` is rejected, redoing consent via the existing reauthorize flow cannot recover the connection because the authorize request and code exchange both fail with `invalid_client`. A new `ReregisterDynamicClient` method and a dedicated `/api/mcp/client/{id}/reregister` endpoint register a replacement client first, then run consent against it. ## Changes - Extracted a shared `registerDynamicClient` helper in `discovery.go` that both `InitiateOAuthFlow` (bootstrap) and `ReregisterDynamicClient` (replacement) call with identical client metadata, ensuring redirect URIs and grant types are consistent across registrations. Credentials are stored as literal `SecretVar` values rather than going through `NewSecretVar`, preventing an `env.`/`vault.`-prefixed `client_id` from being misinterpreted as a secret reference. - Added `ReregisterDynamicClient` on `OAuth2Provider`: loads the stored oauth config, calls the registration endpoint, and persists the new credentials via `RotateMCPOAuthConfig`, which cascades every token bound to that config to `needs_reauth` in the same operation. This cascade is intentional — tokens issued under the previous `client_id` cannot be refreshed once it is replaced, including end-user tokens on `per_user_oauth` servers. - Added `POST /api/mcp/client/{id}/reregister` alongside the existing `reauthorize` route. Both share a `startMCPClientReauthorization` body; the new route passes `reregisterClient=true` to trigger registration before the flow is initiated, so the authorize URL returned to the admin already carries the newly registered `client_id`. The response includes `registered_client_id` and `previous_client_id` fields when a registration occurred. - Kept re-registration as an explicit operator action rather than an automatic fallback on `invalid_client`. Authorization servers do not report RFC 6749 error codes consistently enough to safely trigger credential replacement, and silently discarding a manually configured `client_id` would be worse than leaving it in place. - Added `dynamic_client_reregistration_test.go` with a SQLite-backed config store double and an evictable in-memory authorization server that simulates a registry wipe. Tests cover: recovery after eviction, plain reauthorize not touching the registration, cascade to all bound tokens, error when no registration URL is configured, and error on an unknown config ID. ## Type of change - [x] Bug fix - [x] Feature ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) ## How to test ```sh go test ./framework/oauth2/... -run TestReregister go test ./framework/oauth2/... -run TestReauthorize_WithoutReregistering go test ./framework/oauth2/... -run TestReregisterDynamicClient go test ./... ``` To exercise the endpoint manually: 1. Configure an MCP client against an OAuth provider that supports RFC 7591 dynamic registration and no static `client_id`. 2. Restart the authorization server so it forgets its client registry. 3. Trigger a token refresh — the token should flip to `needs_reauth` with `invalid_client` in the reason. 4. `POST /api/mcp/client/{id}/reauthorize` — the returned authorize URL carries the old, rejected `client_id` (expected dead end). 5. `POST /api/mcp/client/{id}/reregister` — the response includes `registered_client_id` and `previous_client_id`; the authorize URL now carries a `client_id` the provider recognises. ## Breaking changes - [ ] Yes - [x] No ## Related issues Closes maximhq#7191 ## Security considerations - Newly registered `client_id` and `client_secret` values are stored as literal strings, not resolved through the secret-reference mechanism, preventing accidental resolution of provider-issued credentials that happen to carry an `env.` or `vault.` prefix. - Re-registration is gated behind an explicit operator-invoked endpoint rather than triggered automatically, limiting the blast radius of credential replacement to cases where an admin has decided it is necessary. - `RotateMCPOAuthConfig` cascades all bound tokens to `needs_reauth` atomically, ensuring no token issued under the replaced client can be silently used after the swap. ## 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
… client when provider rejects the existing `client_id` (maximhq#7229) ## Summary When an OAuth provider invalidates the client registration it originally issued to Bifrost (returning `invalid_client` on token refresh), a plain Reauthorize cannot recover the connection because it reuses the same rejected `client_id`. This PR adds a "Reauthorize with a new client" action that registers a replacement OAuth client with the provider before running the consent flow, allowing the connection to be restored in cases where the provider no longer recognises Bifrost's client. ## Changes - Added a `reregisterMCPClient` API mutation that calls the new `/mcp/client/:id/reregister` endpoint, which registers a replacement OAuth client and then drives the same consent flow as a normal reauthorize. The endpoint is kept separate from reauthorize rather than hidden behind a flag because it signs out every credential bound to the server config — including all end-user credentials on `per_user_oauth` servers. - Added a "Reauthorize with a new client" entry to the MCP client actions menu, gated behind an `AlertDialog` confirmation that explains the sign-out impact before proceeding. - Added a `providerRejectedTheClient` utility that detects `invalid_client` in a credential's `status_reason` string, and used it in the credential detail view to surface an additional hint when a plain reauthorize will fail for the same reason. - Added unit tests for `providerRejectedTheClient` covering the `invalid_client` match, case-insensitivity, `invalid_grant` non-match, and absent/empty reasons. - Extended `InitiateMCPClientVerificationResponse` with `registered_client_id` and `previous_client_id` fields returned only by `/reregister`, so callers can report which client the consent flow belongs to. ## 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 an MCP server with OAuth and simulate a provider rejecting the client by setting the credential `status_reason` to a string containing `invalid_client`. 2. Verify the credential detail view shows the additional hint alongside the `needs_reauth` badge. 3. Open the actions menu for the server and confirm "Reauthorize with a new client" appears for OAuth and `per_user_oauth` servers. 4. Select it and verify the confirmation dialog appears, describing the sign-out impact correctly for both auth types. 5. Confirm the dialog and verify the reregister endpoint is called and the OAuth consent popup opens. 6. Verify that plain "Reauthorize" still calls the original endpoint and does not trigger the confirmation dialog. ```sh cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` ## Screenshots/Recordings Before: Only "Reauthorize" was available; no guidance was shown when the provider had rejected the client itself. After: A "Reauthorize with a new client" action appears in the menu with a confirmation dialog warning about sign-out impact. The credential detail view shows an additional hint when `invalid_client` is detected in the rejection reason. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations The reregister action discards the existing OAuth client registration and all tokens bound to it. The confirmation dialog makes this impact explicit before the user proceeds. The action is gated behind `hasUpdateAccess`, consistent with the existing reauthorize action. ## Checklist - [x] 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
… section for invalid/dropped DCR client recovery (maximhq#7230) ## Summary Adds a `POST /api/mcp/client/{id}/reregister` endpoint and corresponding documentation for recovering an MCP OAuth connection when the upstream provider no longer recognises the `client_id` it originally issued. This is distinct from plain reauthorization, which reuses the stored `client_id` and therefore fails the same way the background refresh did. The new endpoint runs Dynamic Client Registration (RFC 7591) to obtain a fresh client, writes it over the stored credentials, and then drives the normal consent flow against it. ## Changes - Added `POST /api/mcp/client/{id}/reregister` to the OpenAPI spec (`openapi.yaml` and `mcp.yaml`), with full error documentation (400 when no `registration_url` exists or the provider rejects registration, 404, 503, 500) - Documented the new endpoint in `oauth.mdx` under a dedicated **Reauthorizing with a new client** section, including a warning that replacing the `client_id` invalidates all tokens bound to the OAuth config — including every end-user credential on a `per_user_oauth` server - Added a new troubleshooting accordion for `invalid_client: Invalid client_id` errors where plain reauthorization does not help - Updated the existing `Invalid redirect URI` warning and troubleshooting entry to point at the new endpoint instead of the manual delete-and-recreate instruction - Added the new endpoint to the API reference table in `oauth.mdx`; the response carries `registered_client_id` and `previous_client_id` in addition to the standard `pending_oauth` payload so callers can confirm which client the consent flow belongs to The endpoint is intentionally separate from `reauthorize` rather than a flag on it, because the two differ in what they destroy and that distinction needs to be visible to access control and audit logging. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [x] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [x] Docs ## How to test 1. Configure an MCP client with an OAuth provider that uses Dynamic Client Registration. 2. Restart the provider so its in-memory client registry is cleared. 3. Observe the credential enter `needs_reauth` with a status reason containing `invalid_client`. 4. Confirm that `POST /api/mcp/client/{id}/reauthorize` fails at the code exchange with the same `invalid_client` error. 5. Call `POST /api/mcp/client/{id}/reregister` and verify the response includes `registered_client_id`, `previous_client_id`, `authorize_url`, `status_url`, and `complete_url`. 6. Complete the consent flow: open `authorize_url`, poll `status_url` until `authorized`, POST `complete_url`. 7. Confirm the credential is back in an authorized state and background token refresh succeeds against the new `client_id`. 8. On a `per_user_oauth` server, confirm that end-user credentials are invalidated and users are prompted to sign in again. ## Breaking changes - [ ] Yes - [x] No ## Security considerations The endpoint replaces stored OAuth client credentials (`client_id` / `client_secret`) and invalidates all tokens bound to the previous client. On `per_user_oauth` servers this forces every end user to re-authenticate. Access to the endpoint should be gated by the same management auth as `reauthorize` (bearer token or basic auth), and the intentional separation from `reauthorize` ensures the more destructive action is distinct in access control policies and audit logs. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [x] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
…t_name`) from `varchar` to `text` to stop Postgres rejecting long values with `server_error` (maximhq#7405) ## Summary Fixes a production bug where any OAuth2 `/oauth2/authorize` request with a `state` parameter longer than 512 characters was rejected with an opaque `server_error` on Postgres-backed deployments. The root cause was that `oauth2_authorize_requests.state` was declared `varchar(512)`, and the same class of problem applied to `oauth2_authorize_requests.scope`, `oauth2_clients.client_name`, `oauth2_clients.scope`, and `oauth2_refresh_tokens.scope` (all `varchar(255)`). Postgres enforces these bounds; SQLite does not, so the failure was invisible in development and only surfaced in production. A Gemini Enterprise connector sending a 620-character base64 JSON blob as `state` could never complete per-user authorization. RFC 6749 §4.1.1 places no bound on `state`, §3.3 places none on `scope`, and RFC 7591 §2 places none on `client_name` or `scope`. All five columns are now `text`. The effective upper bound remains the request header block (`server.read_buffer_size`). ## Changes - `oauth2_authorize_requests.state` changed from `varchar(512)` to `text` in the struct tag. - `oauth2_authorize_requests.scope`, `oauth2_clients.client_name`, `oauth2_clients.scope`, and `oauth2_refresh_tokens.scope` changed from `varchar(255)` to `text` in the struct tags. - A new database migration `widen_oauth2_client_controlled_columns` issues `ALTER TABLE … ALTER COLUMN … TYPE TEXT` for each of the five columns on Postgres. SQLite has no `ALTER COLUMN TYPE` and never enforced the bounds, so the migration is a no-op there. Fresh installs already create these columns as `text` from the updated struct tags, so the `ALTER`s are harmless on new databases. - Unit tests added to `migrations_test.go` verify that pre-migration inserts fail on Postgres (proving the bound existed), the migration succeeds, post-migration inserts with 4096-character values succeed, and re-running the migration is a no-op. - Unit tests added to `rdb_oauth2_test.go` verify that long `state`, `client_name`, and `scope` values round-trip verbatim through the store layer on both SQLite and Postgres, and that the column type reported by the driver is `text`. - Handler-level tests added to `mcpoauth2issuance_test.go` verify that a 620-character `state` and a 200-token `scope` each reach the consent redirect without error and are stored verbatim. - E2E collection updated with a new group (case 39) covering DCR with a 1024-character `client_name` and a 200-token scope, and two authorize flows with long `state` and `scope`, asserting a `302` to `/oauth/consent?flow=` with no `error=` in the redirect. ## Type of change - [x] Bug fix ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) ## How to test ```sh go test ./framework/configstore/... -run TestMigrationWidenOAuth2ClientControlledColumns go test ./framework/configstore/... -run TestCreateOAuth2AuthorizeRequest_LongStateRoundTrips go test ./framework/configstore/... -run TestOAuth2ClientControlledFields_LongValuesRoundTrip go test ./transports/bifrost-http/handlers/... -run TestHandleRegister_DCR go test ./transports/bifrost-http/handlers/... -run TestHandleAuthorize ``` For Postgres-specific validation, set `POSTGRES_DSN` to a reachable instance before running. The migration tests will automatically include the Postgres backend and assert that pre-migration inserts are rejected and post-migration inserts succeed. ## Breaking changes - [x] No ## Security considerations The widened columns accept values up to the HTTP request header size limit (`server.read_buffer_size`). No unbounded server-side allocation is introduced; the existing request parsing layer remains the effective cap. The `state` parameter is a client-supplied CSRF token and is not interpreted server-side, so widening it does not expand the attack surface beyond what was already accepted at the HTTP layer. ## 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
… customers (maximhq#7251) ## Summary Extends the grant framework to support access profiles attached to teams, business units, and customers — mirroring the existing user access profile pattern. This allows limits and permits derived from entity-level profiles to be tracked and reported distinctly, so refusals can identify whose profile was exhausted or which entity's profile admitted a request. ## Changes - Added `LimitHolderTeamAccessProfile`, `LimitHolderBusinessUnitAccessProfile`, and `LimitHolderCustomerAccessProfile` holder kinds (along with their `ProviderConfig` and `ModelConfig` variants) to `limit.go`, so spending against an entity's access profile is attributed separately from a user's profile. - Added `PermitTeamAccessProfile`, `PermitBusinessUnitAccessProfile`, and `PermitCustomerAccessProfile` permit types to `permit.go`, each with a corresponding `PrettyString` rendering, so refusals can name the specific entity whose profile was involved rather than giving a generic "access profile" message. - Updated the comment on `LimitHolderUserAccessProfile` to reflect that the anticipated future case (profiles attachable to non-user entities) has now arrived and is handled by the new kinds. - Added test cases for the three new permit type `PrettyString` values. ## 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 ./framework/grant/... ``` Verify that the three new permit types return the correct pretty strings (`"team access profile"`, `"business unit access profile"`, `"customer access profile"`) and that the new limit holder kind constants are accessible and correctly valued. ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations Limit holders and permit types are used in refusal messages and spend attribution. The new kinds are additive and follow the same pattern as existing user-scoped kinds, so no existing attribution or access control logic is affected. ## 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
…mhq#7253) ## Summary Adds `business_unit_id` as a third ownership option for virtual keys, alongside the existing `team_id` and `customer_id`. A virtual key may belong to at most one owner; this PR extends that constraint to cover business units without introducing a foreign key or GORM association, since business units are an enterprise table outside this module's scope. ## Changes - Added `BusinessUnitID *string` to `TableVirtualKey` with a bare indexed varchar column (no foreign key), so the column records ownership without this side needing to resolve what a business unit is. - Extended the `BeforeSave` mutual-exclusion check from a single team/customer pair comparison to a counted check across all three owners, so adding a future fourth owner cannot silently leave a pair unguarded. - Updated `GenerateVirtualKeyHash` to include `businessUnitID` in the hash only when set, preserving existing hashes for keys that predate business-unit ownership and avoiding config-sync drift. - Added a database migration (`add_virtual_key_business_unit_column`) that adds the column and an index to `governance_virtual_keys`, with a rollback path that drops both. The index is required because the column is walked per request to find the key's owner. - Extended `CreateVirtualKeyRequest` and `UpdateVirtualKeyRequest` in the HTTP handler to accept `business_unit_id`. The create and update paths validate the single-owner constraint before reading or writing anything. - Refactored `applyVirtualKeyOwnershipUpdate` to use a `switch` over all three owners: naming one clears the other two, and naming more than one is rejected. Extracted `virtualKeyOwnerCount` and `namedVirtualKeyOwners` helpers to make the count-based check reusable across create and update. ## Type of change - [ ] Bug fix - [x] Feature - [ ] 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/... ``` - `TestMigrationAddVirtualKeyBusinessUnitColumn` verifies the column and index are created on a pre-migration table and that re-running the migration is a no-op. - `TestVirtualKeyOwnerMutualExclusion` exercises every pair (team+customer, team+business unit, customer+business unit, all three) and confirms each is rejected, while each single-owner and no-owner case is accepted. - `TestApplyVirtualKeyOwnershipUpdateBusinessUnit` confirms that setting a business unit on a key that previously had a team clears the team, and that naming a business unit alongside a customer is refused. ## Breaking changes - [ ] Yes - [x] No ## Security considerations `business_unit_id` is stored as a plain string with no server-side validation of whether the referenced business unit exists. Validation is the responsibility of the caller that owns the business-unit table. No secrets or PII are introduced; the field participates in the virtual key hash only when set, using the same write-only-when-present pattern as the existing owner fields. ## 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
…maximhq#7254) ## Summary In Enterprise mode, customer keys are governed by access profiles rather than per-customer budgets and rate limits. This PR ensures the customer creation/editing form respects that distinction by hiding limit-related fields and omitting limit data from API calls when running in Enterprise mode. ## Changes - Introduced an `editsLimits` flag (derived from `!IS_ENTERPRISE`) that controls whether budget and rate limit fields are rendered and submitted. - When `editsLimits` is `false` (Enterprise), editing a customer sends only the `name` field, preventing accidental overwrites of any limits that may exist on legacy customers. - When creating a customer in Enterprise mode, the create request is similarly restricted to `name` only, omitting `calendar_aligned`, `budgets`, and `rate_limit`. - The budget reset prompt is skipped in Enterprise mode since budget fields are not editable. - The `MultiBudgetLines`, `NumberAndSelect` (token/request limits), and calendar alignment toggle are all conditionally rendered behind `editsLimits`, so they do not appear in the Enterprise UI. ## 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. Run the UI in Enterprise mode (`IS_ENTERPRISE=true`). 2. Open the customer creation sheet and confirm that budget limits, token/request limits, and the calendar alignment toggle are not visible. 3. Create a customer and verify the API request body contains only `name`. 4. Open an existing customer's edit sheet and verify only the name field is present and the update request sends only `name`. 5. Run the UI in non-Enterprise mode and confirm all limit fields remain visible and functional as before. ```sh cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` ## Screenshots/Recordings Before/after screenshots recommended showing the customer sheet in Enterprise vs. non-Enterprise mode. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations Prevents Enterprise deployments from inadvertently overwriting access-profile-governed limits via the customer form. ## 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
…ofile governs it (maximhq#7255) ## Summary Adds business unit ownership to virtual keys, allowing a key to be assigned to a business unit (alongside the existing team and customer options). When a team, customer, or business unit holds an access profile, the virtual key sheet now detects that profile and hides the key's own provider, budget, rate-limit, and MCP editors — sending a stripped payload on save so the server's governance logic is not bypassed. ## Changes - Added `business_unit_id` to `VirtualKey`, `CreateVirtualKeyRequest`, and `UpdateVirtualKeyRequest` types. - Extended the virtual key sheet's `entityType` enum to include `"business_unit"`, wired up a `BusinessUnitPicker` registry (mirroring the existing `UserPicker` pattern), and added the corresponding form field and owner-clearing logic. - Introduced `useGetEntityAccessProfileQuery` in the OSS fallback API (always returns `undefined`) and the matching types (`AccessProfileEntityKind`, `EntityAccessProfile`, `GetEntityAccessProfileResponse`) so OSS and enterprise builds share the same call sites. - Added `withoutKeyGovernance` helper that strips provider configs, MCP configs, budgets, rate limits, and calendar alignment from a create/update payload before sending it when the selected owner holds an access profile. - Resolved a display bug in `resolveDisplayRateLimit`: keys owned by a team, customer, or business unit (with no assigned user) are now flagged as `isEntityGoverned`, causing the hook to return `vk.rate_limit` directly rather than trying to resolve a user profile rate limit. - Added `useEntityProfileLimits` OSS fallback (returns `{}`) and wired it into the customers table so rows governed by an entity access profile display that profile's budgets and rate limit instead of the customer's own. - Added `EntityAccessProfileSection` OSS fallback components (`EntityAccessProfileField`, `EntityUsage`) that render nothing, keeping enterprise sheet surfaces from needing conditional imports. - Added a `businessUnitPicker` registration module (OSS fallback is a no-op export; enterprise build registers the real picker) and imported it as a side-effect in the virtual key sheet. - Added `EntityAccessProfiles` as a separate RTK Query tag so attaching a profile to one entity does not invalidate the full access-profile template list. - The virtual keys table now shows "Business unit" in the assigned-to cell for keys with a `business_unit_id` rather than leaving it blank. ## 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. Open the virtual key sheet and confirm "Assign to Business Unit" appears only in enterprise builds (where the picker is registered). 2. Create a virtual key assigned to a business unit that holds an access profile; verify the provider, budget, rate-limit, and MCP editors are hidden and the saved payload omits those fields. 3. Edit an existing key owned by a profile-holding team or customer; confirm the same editors are hidden and the governance notice names the profile. 4. Confirm OSS builds compile and render without errors (the fallback modules resolve to no-ops). 5. In the customers table, verify that a customer governed by an access profile shows the profile's budgets and rate limit in its row. ```sh cd ui pnpm i pnpm test pnpm build ``` ## Screenshots/Recordings _Add before/after screenshots of the virtual key sheet showing the business unit owner option and the governance lock notice._ ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations Business unit ownership follows the same RBAC checks already applied to team and customer ownership. The `withoutKeyGovernance` helper ensures governance fields are never sent when an entity profile is in effect, preventing a client from bypassing server-side spend controls by submitting its own limits. ## 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
…erns it (maximhq#7386) ### TL;DR Prevents teams and customers from simultaneously holding their own budgets/rate limits and being governed by an access profile, which would result in duplicate caps on the same keys. ### What changed? A new `LegacyLimitGuard` mechanism was introduced in the governance plugin that allows the enterprise build to register a callback answering whether a given team or customer is already governed by an access profile. In OSS builds, no guard is registered and nothing is refused. The team and customer update handlers now call `LegacyLimitsGovernedBy` before writing budgets or rate limits. If the entity is already governed by an access profile, the request is rejected with a 409. If the lookup fails, the request is rejected with a 503 rather than silently allowing a second cap to be written. The config reconciler (`updateGovernanceConfigInStore`) applies the same check when processing `governance.budgets` from `config.json`. A budget whose owning team or customer has since been moved onto a profile is skipped with a warning rather than causing the reconcile to fail, since the stale config entry shouldn't block the rest of the file from applying. A failed lookup does stop the reconcile. Two bugs in `coerceLegacyBudget` were also fixed: `ResetConfig` is now carried forward when merging a partial budget update (previously, a partial edit would silently reset a quarterly budget's fiscal start to the default), and `isBudgetRemovalRequest` now correctly treats a request carrying only `ResetConfig` as an edit rather than a removal. ### How to test? - In an OSS build, update a team or customer with budgets or a rate limit and confirm the request succeeds as before. - Register a `LegacyLimitGuard` that returns a profile name for a specific entity, then attempt to set a budget or rate limit on that entity via the API and confirm a 409 is returned naming the profile. - Confirm that a guard returning an error causes the API to return a 503. - Update a team or customer with no budgets and no rate limit maximums while a guard is registered and confirm the guard is not consulted. - Send a partial budget update (e.g., only `max_limit`) to a customer with a quarterly budget and confirm `reset_config` is preserved. ### Why make this change? A team or customer governed by an access profile and also carrying its own budgets or rate limit ends up with two independent caps on the same API keys. The cap from the entity's own limits is invisible in the profile editor, making it impossible to reason about or manage the effective limit from one place. This change enforces that only one governance mechanism applies to any given entity at a time.
…onfig.json still declares them (maximhq#7401) ## Summary When an access profile governs a team or customer, that entity's budgets and rate limits are owned by the profile. A config.json file that still declares limits for the same entity is stale, and the reconcile must not overwrite what the profile put in place. Previously, the reconcile would apply the file's rate-limit rows before reaching the customer or team that links them, meaning a governed entity's stored rate limit was silently replaced before any check could stop it. Budgets and budget links were similarly overwritten or unlinked. ## Changes - `ScopedDB` now accepts an optional `*gorm.DB` transaction so reads taken inside a transaction see that transaction's own writes. The query scope is applied either way. - `GetTeam` and `GetCustomer` accept the same optional transaction and pass it through to `ScopedDB`, allowing governed-entity reads to be consistent with in-flight writes. - `governedEntitiesInConfig` is introduced and called once before any writes begin. It identifies which teams and customers in the current reconcile are governed by an access profile, and reads the rate-limit ID each one links today. This snapshot is used throughout the write loops. - Rate-limit rows linked by a governed entity are skipped on both create and update, preserving the numbers the profile established. - Governed customers and teams have their `RateLimit`, `RateLimitID`, and `Budgets` fields replaced with the stored values before the row is saved, so the upsert leaves those columns unchanged. - Budget-link and budget-unlink paths skip governed customers entirely, preventing the file's silence from unlinking budgets the profile's arrival left in place. - `skipBudgetGovernedElsewhere` is renamed to `skipBudgetOfGovernedEntity` to better describe what it guards. - Two SQLite integration tests cover the new behaviour: one verifies a governed customer's stored budgets survive a reconcile that declares different ones, and one verifies the rate-limit row it links is not overwritten when the file raises the cap. ## Type of change - [x] Bug fix ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [x] Plugins ## How to test ```sh go test ./framework/configstore/... ./transports/bifrost-http/... ``` The two new tests exercise the governed-entity paths end-to-end against a real SQLite database: - `TestSQLite_Customer_GovernedByProfileKeepsItsStoredBudgets` — confirms that a governed customer's existing budget is not deleted or unlinked, and that the file's replacement budget is not created. - `TestSQLite_Customer_GovernedByProfileKeepsItsStoredRateLimit` — confirms that the rate-limit row a governed customer links is not updated when the file raises the cap. ## Breaking changes - [x] Yes `GetTeam`, `GetCustomer`, and `ScopedDB` on the `ConfigStore` interface now accept a variadic `*gorm.DB` argument. Any external implementation of `ConfigStore` must add that parameter. Callers that pass no transaction are unaffected. ## Security considerations The change tightens isolation between access-profile-governed limits and file-declared limits. Without it, a config.json edit could silently replace rate limits that an access profile was meant to enforce, potentially widening spend caps beyond what the profile intended. ## 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
## Summary Enforces RBAC permissions across several UI surfaces so that users without the appropriate role cannot create, edit, or delete resources they are not authorized to change. Previously, action buttons and controls were always rendered and enabled regardless of the viewer's permissions. ## Changes - **Caching view**: The enable/disable toggle and the "Save Changes" button are now disabled when the viewer lacks `Settings.Update` permission. Both controls are wrapped in a `Tooltip` that explains the restriction on hover. - **Pricing overrides**: The "New Override" button and the empty-state "Create Override" button are hidden when the viewer lacks `Settings.Create` permission. The edit and delete actions in the row actions menu are hidden individually based on `Settings.Update` and `Settings.Delete` permissions respectively. The save button inside the override sheet is disabled and carries a `title` tooltip when the viewer cannot save. `PricingOverridesEmptyState.onCreateClick` is now optional so the empty state can be shown without offering a creation action. - **Plugin tracing sheet**: The save button is disabled with a `title` tooltip when the viewer lacks `Observability.Update` permission. - **OTel view**: The "Configure Tracing" button is hidden entirely when the viewer lacks `Observability.Update` permission. - **Skills repo version dialog**: The "Shift Version" button is hidden when the viewer lacks `SkillsRepository.Update` permission. - **Prompt repository sheets** (`commitVersionSheet`, `folderSheet`, `promptSheet`): Save/commit buttons are disabled with `title` tooltips when the viewer lacks the relevant `PromptRepository` create or update permission. ## 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 ```sh cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` Log in as a user with a read-only role and verify: - The caching toggle and save button are disabled and show a tooltip explaining the restriction. - The pricing overrides table shows no "New Override" button and no edit/delete actions in the row menu. - The empty state for pricing overrides renders without a "Create Override" button. - The plugin tracing save button is disabled with a tooltip. - The "Configure Tracing" button in the OTel view is not rendered. - The "Shift Version" button in the skills repo version dialog is not rendered. - Commit, folder, and prompt save buttons are disabled with tooltips in the prompt repository. Log in as a user with full access and verify all controls remain fully functional. ## Breaking changes - [ ] Yes - [x] No ## Security considerations Permission checks are enforced client-side via the existing `useRbac` hook. These changes prevent unauthorized UI interactions but do not replace server-side authorization, which must remain the authoritative enforcement layer. The pricing override sheet also re-checks permissions at save time to guard against stale pages or deep links that bypass the hidden entry points. ## 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
…governance (maximhq#7421) ## Summary Adds `business_unit` as a new entity type that can hold governance budgets and rate limits, alongside the existing `team` and `customer` types. Unlike teams and customers, a business unit may hold at most one budget. ## Changes - Added `LegacyLimitHolderBusinessUnit = "business_unit"` constant to the governance plugin - Updated the comment block to clarify the distinction between entity types: teams and customers may hold several budgets, while a business unit holds at most one ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [x] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./plugins/governance/... ``` Verify that `business_unit` is accepted as a valid limit holder type in governance budget configuration and that it correctly enforces the single-budget constraint. ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations No security implications. This change extends the set of entity types that can be governed, which narrows rather than broadens spend 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
…usiness units (maximhq#7432) ## 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 maximhq#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
…mhq#7366) ## Summary Token counts reported by the logging plugin and the tracer were derived from separate, duplicated switch blocks, creating the possibility of the two diverging. This PR introduces a single `NormalizedUsage()` method on `BifrostResponse` that all callers can use to extract a canonical `BifrostLLMUsage` from whichever response family is populated, and replaces the inline switch in the logging plugin with a call to it. ## Changes - Added `BifrostResponse.NormalizedUsage()` to `core/schemas/mux.go`, which handles all response variants (chat, text completion, responses, compaction, embedding, rerank, transcription, speech, image generation, and passthrough) in one place. - Removed the duplicated usage-extraction switch from `plugins/logging/operations.go` and replaced it with `result.NormalizedUsage()`, ensuring the logging path and the span/tracer path read from the same source and cannot report different token counts for the same request. - The method intentionally does not cover streaming responses (those settle through the accumulator) or cost/billing extraction (which lives in `framework/modelcatalog/datasheet` and needs additional context like service tier, audio seconds, and image size). ## Type of change - [ ] Bug fix - [x] Refactor - [ ] Feature - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [x] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./... ``` Verify that token counts in logs and traces match for chat, embedding, transcription, speech, image generation, and passthrough requests. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations None. ## 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
…c check Upstream maximhq#7253 added TableVirtualKey.BusinessUnit (gorm:"-", resolved on read from business_unit_id) without excluding it from TestConfigSchemaSync, which fails on upstream ec081ef. Upstream later excluded it (c649d08); this takes the same exclusion. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…dler tests After startBackfill the sidekiq runner's goroutine reads the fake store, while the tests assigned its jobs map and inFlight job directly. go test -race caught it once in TestWarpAdminRoutesAdmitRBACAuthorizedCaller during a full transports run. The tests now go through locked helpers; the race remains on upstream's dev tip. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A prerelease version: the hyphen keeps the GitHub release a prerelease and the latest and latest-ubi9 image tags on v2.2.6, and 2.3.0 stays free for the complete upstream v2.3.0 checkpoint. The changelog names the checkpoint, what it brings, its migrations and the fork's notes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
validate-schema-sync.sh gates the release pipeline and failed on TableVirtualKey.BusinessUnit, the gorm:"-" display object upstream maximhq#7253 added. Upstream excludes it the same way later on dev (c649d08). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
Security findingsAdvisory findings (1)ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Upstream maximhq#7367 moved span attributes onto the typed LLMSpanData renderer, whose Responses branch never emits finish_reasons, so a /v1/responses refusal reached no connector. That undid upstream maximhq#7205 and failed the fork's TXE-5038 check in run-observability-local.mjs (no span with gen_ai.response.finish_reason). The renderer now emits the stop_reason the span builder already records; upstream's dev tip still drops it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Upstream maximhq#7257 hides deprecated models from an unsearched GET /api/models. The datasheet now marks the stub's only Anthropic model, claude-haiku-4-5, deprecated, so the case lost Anthropic and failed although the VK filter was right. It asks for deprecated models too and still checks the VK filter. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Upstream maximhq#7491's matrix calls /api without the setup token the fork requires since v2.2.6, and requires pg, which the release pipeline's API job never installs. It now sends the token as the other runners do; pg@8.13.0 is pinned in .github/workflows/tools beside newman, installed by the job's existing npm ci, and the runner resolves it from newman's node_modules. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Upstream's API job now runs the Warp newman suite, which answers with a live OpenAI model. The fork's job has no provider keys, so the suite failed the run; it is skipped, with a notice, unless the job brings OPENAI_API_KEY or WARP_UPSTREAM_BIFROST. The key is read before the list-models stub exports its placeholder. Its seeder also builds outside go.work, which does not list tests/cmd/seed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A key that happens to be present no longer starts it: the release pipeline makes no live provider calls, so the Warp newman stage runs only with BIFROST_E2E_RUN_LIVE_WARP=1. Offline Warp coverage stays in the framework/warp and handler Go tests; the deterministic-stub follow-up is TXE-5175. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2dbc1ea628
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…g path Upstream maximhq#7367 renders span attributes only from the typed LLMSpanData record, which models inference families but not batch or file operations, so their attributes (batch and file ids, status, input file, filenames, sizes) stopped reaching connectors. The tracer now merges the PopulateBatch*/PopulateFile* attributes for those families, without overriding typed keys. Codex connector P2 on PR #28. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f575be465c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ameters The typed span renderer chose the text-completion branch on its parameters, so a request without them (valid through the Go SDK) lost gen_ai.input.text, which the attribute helpers this replaced always emitted. The branch now also runs for a prompt alone, with the parameters rendered only when present. Codex connector P2 on PR #28. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Upstream's dependabot sweep (maximhq#7670) moved the five example MCP servers' fast-uri override to 3.1.7, which is still inside GHSA-hrr3-gc8f-f4qj, GHSA-wjr9-j795-jxcr, GHSA-w74x-v6f9-j3xg, GHSA-333x-w4r6-6crp and GHSA-xhq7-xmqj-769h (fixed in 3.1.9), so dependency-review failed. These servers only back the MCP tests. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The same dependabot sweep moved ip-address to 10.7.2, inside GHSA-rxp7-6hxx-x8q5 (quadratic zone-identifier matching, fixed in 10.7.4). It is transitive here (^10.2.0), so the lock entries move; npm ci installs 10.7.4 in all five. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df0bf38951
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…oads Upstream maximhq#7391 exports raw provider bodies to connectors that opt in and redacts them with literal guardrail replacements, but a JSON body carries that text escaped, so a value with a quote, backslash, newline or HTML character kept its original content in the exported body while the typed fields were masked. A JSON body is now redacted on its decoded string values and keys and re-encoded, only when something changed; a non-JSON body (an SSE stream) also gets the JSON-escaped forms of each replacement. Codex connector P1 on PR #28. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… raw body The decoded-value redaction read numbers as json.Number and skipped them, so a matched number (an account or phone number) stayed in the exported raw body, which the literal pass on the bytes used to replace. A matched number now becomes the replacement string. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70806a58c6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A Warp tool result becomes a function-call output in the next model prompt, and sonic.MarshalString orders map keys arbitrarily, so equal results could produce different prompts and miss the prompt cache. boundToolResult now uses providerUtils.MarshalSorted, as AGENTS.md requires. Codex connector P1 on PR #28. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Upstream's checker reconnect tests move into connectionchecker_test.go, which already covers ClientConnectionChecker.performCheck, and this PR's own span tests take the names of the files they cover (spanattrs_test.go, spandata_test.go), per AGENTS.md's one-test-file-per-source rule. Codex connector P1 on PR #28. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57d685a06c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…cover the code AGENTS.md keeps one test file per source file. The upstream slice adds eleven feature-named test files beside existing coverage; their tests and fixtures move into the existing file for the code they exercise (bifrost_test.go, clientmanager_test.go, connectionchecker_test.go, toolmanager_test.go, health_monitoring_test.go, main_test.go, mcpserver_test.go), or the file takes its source's name where none existed (rdb_test.go, mcpoauth2_test.go). Every test function is kept; the packages pass. Codex connector P1 on PR #28. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c62f79af4b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
rejectInsecureRedirect compared Hostname(), which drops the port, so a gateway redirect to https://gateway.example:9443 was followed and the client forwarded x-bf-vk to that separate origin. A same-scheme redirect must now keep the effective port; writing out the scheme's default port is still the same origin. Codex connector P2 on PR #28. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The server files a failed first turn and names its thread on the error frame, but the panel kept the id only from done, so the next question opened a second thread. Both terminal frames now set it, through terminalConversationID. Codex connector P2 on PR #28. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The port check skipped redirects that change scheme, so http://host could redirect to https://host:9443, another origin, and receive the credentials. A port change is now refused unless it is only the implicit default-port switch of a bare scheme change (http://host to https://host). Found by an automated security review of 75ad63e. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🛡️ Codex Security Review · Automatically triggered
Here are some automated security review suggestions for this pull request.
Reviewed commit: 84c8b1e7d3
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
Comment on lines
+39
to
+43
| scope, ok := raw.(queryscope.QueryScope) | ||
| if !ok || scope == nil { | ||
| return nil, func() {}, fmt.Errorf("query scope is present but unusable (%T); refusing to run Warp unscoped", raw) | ||
| } | ||
| base = queryscope.WithQueryScope(base, scope) |
There was a problem hiding this comment.
🛡️ Codex Security Review · Automatically triggered
When Warp is enabled in an enterprise deployment, a user with WarpSession View can ask describe_filter_space or query_usage_by about organization dimensions. snapshotWarpContext copies only BifrostContextKeyQueryScope; it omits BifrostContextKeyDimensionScope, so applyDimensionCeiling treats the agent context as unrestricted. Row scope is insufficient by design: one legitimately visible multi-affiliation log can carry a customer or business unit the caller may not see, exposing its ID/name and attributed request, token, and cost totals. Copy and fail closed on the dimension scope exactly as for QueryScope. The flag is default-off, so this path is conditional.
SECURITY.md reference: SECURITY.md:L83-L88
Useful? React with 👍 / 👎.
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
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.
First bounded slice of the upstream v2.3.0 development line (epic TXE-4803). It merges upstream dev
ec081ef5bb98ca43e8e75de157dcbf76662cc96fonto fork mainb5eb6c82c(v2.2.6) and releases as the prerelease transport v2.3.0-sync.1.It is a real
git merge(bac398794, parentsb5eb6c82c+ec081ef5b), with no squash and no-s ours, soec081ef5bbecomes an ancestor of main. The merge base is6a622ad52(upstream v2.2.3 + 12, which dev back-merged at its index 112).Checkpoint: dev first-parent commits 1-173 after v2.2.6's base
8b4fce4f1.ec081ef5bis the last dev commit before dev's squashed v2.2.4 import (1ef2bf2ac). The coordinator approved it over the largereeb5677a9candidate:048492cffec081ef5b(this PR)eeb5677a9c649d08feUpstream v2.3.0 itself (merge v2.3.0 maximhq/bifrost#8399,
05e61aaff) is not merged. Stable2.3.0stays reserved for that checkpoint.What this slice brings
MCP reconnect/refresh-tools/re-registration; business-unit owned virtual keys; connector exports (typed span payloads, cost breakdown, opt-in raw payloads, metric dimensions); the
bifrostCLI management and inference commands; unified model and provider selectors; session-grouped logs; pinned keys on routing fallbacks; per-attempt TTFT cutoffs; failure classes andretry_after_ms; per-VK content-logging control; VK auto-deletion after expiry; Typesafe SDK fidelity; Warp, behind thewarpfeature flag, off by default (verified on the built image). Details and every migration are intransports/changelog.md.Preserved fork behaviour (checked, unchanged by the merge)
/api/providerswith 401 without a token, 403 with a wrong one, 200 with the right one;inference_auth_enforcedistrueon a fresh deployment.PathOriginal).ek_bf_realtime secrets and provider/key binding, native Anthropic and xAI APIs, custom providers (ModelArk, Volcengine), streaming/unary clients and idle timeouts, manager-owned stream state, replay statuses, credential redaction, limits migration, and every PR 20-27 fix: these files merged cleanly or with the fork's side kept, and their tests pass.github.com/capsohq/bifrost/...; core/framework/plugins are built from the workspace and not tagged.Conflict-resolution ledger (61 files)
core/framework/plugin changelogs and versionstests/cmd/seed/go.modkeeps capsohq and adds upstream'sgoogle/uuid.transports/changelog.md,transports/version86faa87b5.core/providers/bedrock/invoke.go(5 hunks)(int, bool), maximhq#7649 follow-up) and its stream-state release oncompleted/incomplete.transports/bifrost-http/integrations/typesafe.gotransports/bifrost-http/integrations/typesafe_test.go(add/add)core/schemas/decision.goNativeResponse.transports/bifrost-http/handlers/middlewares.gotransports/bifrost-http/handlers/governance.gofrom_memoryremoved (DB-backed reads only); upstream had only edited that branch.framework/configstore/migrations.goui/lib/types/config.tsBaseProviderNamesconstant, carrying the fork's full base-provider list.ui/app/workspace/virtual-keys/views/virtualKeySheet.tsxui/app/_fallbacks/.../accessProfileApi.tsrefetchon the creation-policy stub.tests/e2e/api/collections/provider-harness.json(6 hunks)docs/openapi/openapi.jsonbundle.pyfrom the merged sources.Makefile,SKILL.md, docs, e2e page objects, import-only Go conflictsMerge adaptations: every new upstream Go import and go.mod path is rewritten to
github.com/capsohq/bifrost/...; import blocks deduplicated and gofmt-ed.Commits on top of the merge
c515b6224,f1c367fd2,04bfad708: three upstream defects present onec081ef5bitself, fixed the way upstream later fixed them on dev (c649d08fe): the realtime retry-hint test expects the carried 429;business_unit(response-only) is excluded fromTestConfigSchemaSyncand from schemasync (validate-schema-sync.shgates the release pipeline).aa34866e2: the Warp handler tests wrote the fake sidekiq store without its lock while the runner goroutine read it;-racecaught it once. Still present on upstream's tip.86faa87b5: transport version2.3.0-sync.1and changelog.4c19d7c3c: regression fix. Upstream feat: add typed LLM span payload and attribute renderer maximhq/bifrost#7367 moved span attributes onto the typedLLMSpanDatarenderer, whose Responses branch never emitsfinish_reasons, so a/v1/responsesrefusal reached no OTel connector (undoing upstream [fix]: Emit finish_reason on OTEL spans for Responses API requests maximhq/bifrost#7205; the fork's TXE-5038 check inrun-observability-local.mjscaught it). Fixed with a unit test that fails without the fix. Still broken on upstream's dev tip.21814d537: upstream feat(models): hide deprecated models from unsearched listings, sink them below live ones in search results maximhq/bifrost#7257 hides deprecated models from an unsearchedGET /api/models; the datasheet now marks the stub's only Anthropic model deprecated, so the VK-restriction case asks withinclude_deprecated=true(the VK filter assertions are unchanged).7f235eea2: upstream test(e2e): content-logging permutation matrix across key, client, connector and header maximhq/bifrost#7491's content-logging matrix runs in the API job but called/apiwithout the setup token and requiredpg, which the job never installs. It sends the token like the other runners;pg@8.13.0is hash-pinned in.github/workflows/toolsbeside newman (samenpm ci), resolved from newman'snode_modules.7e3666239, then the opt-in commit on top: upstream added a Warp newman stage to the API job that asks a live OpenAI model. The release pipeline makes no live provider calls, so the stage runs only withBIFROST_E2E_RUN_LIVE_WARP=1and otherwise prints a skip notice; a key being present does not start it. Its seeder builds withGOWORK=off, because the fork'sgo.workdoes not listtests/cmd/seed(the H16 class from PR TXE-4803: Merge upstream Bifrost through v2.3.2 #5; no Dockerfile is affected, sincego.workis unchanged). Offline Warp coverage remains: framework/warp and Warp handler Go tests, Warp UI vitest. The unavailable collection (110 requests in 18 folders) and a deterministic-stub plan are TXE-5175.f575be465(connector P2): upstream feat: add typed LLM span payload and attribute renderer maximhq/bifrost#7367's typed span renderer does not model batch/file operations, so their span attributes (batch/file ids, status, input file, filenames, sizes) stopped reaching connectors; the tracer merges the existingPopulateBatch*/PopulateFile*attributes for those families. Mutation-tested.33c4ee4b3(connector P2): the same typed renderer keyed the text-completion branch on its parameters, so a parameterless request (valid via the Go SDK) lostgen_ai.input.text. Mutation-tested. A throwaway comparison of the typed renderer against the legacyPopulate*helpers found no other dropped request or response keys for chat, text, embedding, speech, transcription and Responses.3c17b1465: upstream's dependabot sweep left the five example MCP servers'fast-urioverride at 3.1.7, inside five advisories fixed in 3.1.9 (dependency-review). Bumped;npm ci, the MCP server build and the MCP test suites pass.df0bf3895: the same sweep leftip-addressat 10.7.2 (GHSA-rxp7-6hxx-x8q5, fixed in 10.7.4); lock entries moved to 10.7.4,npm cipasses in all five.53562a31e(connector P1): upstream feat: export raw payloads to connectors maximhq/bifrost#7391's opt-in raw-payload export redacted guardrail matches only as literal bytes, so content with a quote, backslash, newline or HTML character survived JSON-escaped in the exported raw body. JSON bodies are now redacted on decoded strings and keys (re-encoded only on a match; untouched bodies stay byte-identical); non-JSON bodies (SSE) also get the JSON-escaped forms. Mutation-tested.70806a58c: the raw-body redaction also replaces a guardrail match that appears as a JSON number (found by an automated security review of53562a31e). Mutation-tested.0aefaea0f(connector P1): Warp tool results, which become function-call outputs in the next prompt, serialize withproviderUtils.MarshalSorted(AGENTS.md) instead of unorderedsonic. Mutation-tested. Note: this commit also carries the staged test-file moves of the next one.57d685a06(connector P1): upstream's checker reconnect tests move intoconnectionchecker_test.go(AGENTS.md: one test file per source); this PR's own span tests are named after their source files (spanattrs_test.go,spandata_test.go).c62f79af4(connector P1): the slice's eleven other new per-feature test files move into the files covering their code (or take their source's name where none existed); every test function is kept and the packages pass. Later slices will see these as modify/delete conflicts when upstream edits them.75ad63e0f(connector P2, credential handling): the CLI's redirect guard compared host names without ports, so a redirect to another port on the gateway's host receivedx-bf-vk; a same-scheme redirect must now keep the effective port. Mutation-tested.84c8b1e7d: tightened after an automated security review: a scheme-switching redirect may not change port either (only the implicit default-port switch of a barehttp://hosttohttps://host). Mutation-tested.5d0d71ead(connector P2): the Warp panel keeps the thread id from an error frame as well as done, so a failed first turn does not split the history.Release
2.3.0-sync.1passesvalidate_version(dotted prereleases are pinned inversion-format.test.sh). origin has notransports/v2.3.*tag, so detection takes the first-in-track path; the tag and GHCR tag are both new.latest,latest-ubi9and the R2latest/copy, so they stay on v2.2.6.Tests (local, offline: provider secrets unset, no live provider calls)
mcptestswith the MCP servers built; framework 29 packages under-racein an isolated compose project (bifrost-txe5174, own subnet, no host ports); all 12 plugins andcli; transports-race -short.tsc, vitest (61 files, 845 tests), lint (0 errors).transports/Dockerfilebuild. Smoke:/health200,/api/versionv2.3.0-sync.1, setup lock 401/403/200, UI 200,warpflag off.b5eb6c82c), including all 14 new migrations.make test-memorypasses.test-release-gates.py,test-docker-publish-gates.py,test-migration-fixtures.py,codex_review_check_test.sh, egress allowlist, action runtimes, helm schema/templates/config fields and schema sync pass.test-plugin-release-dependencies.pyfails as before (TXE-5146, not wired).validate-go-config-fields.sh(not wired; also fails on main) anddocs/openapi/spec_invariants_test.py(not wired; the same 4 fail on pure upstreamec081ef5b, 3 on main) are pre-existing upstream gaps.Follow-ups (under TXE-4803)
Closes TXE-5174
🤖 Generated with Claude Code