fix: preserve OpenAI encrypted reasoning item ids through the anthropic surface - #5192
Open
abhishekgahlot2 wants to merge 46 commits into
Open
Conversation
|
|
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a provider-aware envelope for encrypted reasoning, preserves OpenAI item IDs through Anthropic replay, gates destination-specific decoding, and tests grouped, streaming, native, summary, and multi-item paths. ChangesEncrypted reasoning replay
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant OpenAI
participant AnthropicConversion
participant AnthropicClient
participant OpenAIRequest
OpenAI->>AnthropicConversion: encrypted reasoning with item_id
AnthropicConversion->>AnthropicClient: redacted_thinking envelope
AnthropicClient->>AnthropicConversion: replay redacted_thinking block
AnthropicConversion->>OpenAIRequest: restored item_id and deferred envelope
OpenAIRequest->>OpenAI: unwrapped ciphertext
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderabbitai
Bot
requested review from
Pratham-Mishra04,
TejasGhatte,
akshaydeo and
danpiths
July 14, 2026 13:18
Contributor
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (4): Last reviewed commit: "fix: preserve OpenAI encrypted reasoning..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
abhishekgahlot2
force-pushed
the
fix/anthropic-openai-encrypted-reasoning-item-id
branch
2 times, most recently
from
July 14, 2026 13:21
afe739a to
4ae1a05
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
akshaydeo
dismissed
coderabbitai[bot]’s stale review
July 14, 2026 17:22
The merge-base changed after approval.
## 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
|
+1 |
…d replace duplicate implementations in `fetch.go` and `skills_serving.go` (maximhq#5246) ## Summary The SSRF protection logic (IP classification and safe dial context) was duplicated across `core/providers/utils/fetch.go` and `transports/bifrost-http/handlers/skills_serving.go`, with the skills handler using a weaker `isPrivateIP` check that missed CGNAT, IPv6 transition addresses, site-local IPv6, broadcast, and interface-local multicast. This PR extracts a single, hardened implementation into `core/network` and replaces both call sites with it. ## Changes - Added `core/network/ssrf.go` with `IsPublicIP` and `SSRFSafeDialContext` as the canonical SSRF-safe dialing primitives. `IsPublicIP` blocks loopback, RFC 1918, CGNAT (RFC 6598), link-local, site-local (RFC 3879), multicast, broadcast, unspecified, and IPv4 addresses smuggled inside IPv6 transition representations (6to4 `2002::/16`, NAT64 `64:ff9b::/96`, NAT64 local-use `64:ff9b:1::/48`). `SSRFSafeDialContext` resolves the host, validates every returned IP against `IsPublicIP`, and dials the first validated IP directly — eliminating the DNS-rebinding TOCTOU window. - Removed the private `isPublicIP`/`embeddedIPv4` functions from `fetch.go` and the weaker `ssrfSafeDialContext`/`isPrivateIP` functions from `skills_serving.go`; both now call `network.SSRFSafeDialContext`. - Moved tests from `core/providers/utils/fetch_test.go` into `core/network/ssrf_test.go`, expanded them to cover the additional blocked ranges (CGNAT, broadcast, interface-local multicast, IPv6 site-local), and added dial-path tests for DNS error propagation, empty resolution, missing port, re-resolution per dial, and the DNS-rebinding TOCTOU guard. ## Type of change - [ ] Bug fix - [ ] Feature - [x] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./core/network/... ./core/providers/utils/... ./transports/bifrost-http/... ``` The `TestSSRFSafeDialContextBlocksLoopbackLiteral` test exercises the exported constructor end-to-end against the real resolver and will fail if the dial gate is not applied. All other tests use a `fakeResolver` and require no network access. ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations The skills handler previously used `isPrivateIP`, which did not block CGNAT addresses (used as pod IPs on EKS), IPv6 site-local (`fec0::/10`), broadcast (`255.255.255.255`), interface-local multicast, or IPv4 addresses wrapped in 6to4/NAT64 IPv6 representations. An admin-controlled `source_url` or a user-controlled fetch URL could have reached internal infrastructure through any of those gaps. The consolidated `IsPublicIP` closes all of them. The DNS-rebinding TOCTOU fix (dial the already-resolved IP directly) was present in `fetch.go` but absent in the skills handler; it is now enforced in both paths via the shared implementation. ## 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
…ecret rotation with Standard Webhooks signing (maximhq#5247) ## Summary Introduces the `config_webhook_endpoints` table and all associated store-layer logic needed to persist, retrieve, update, and rotate secrets for registered webhook endpoints. This is the foundational persistence layer for outbound webhook delivery. ## Changes - Added `TableWebhookEndpoint` model (`framework/configstore/tables/webhooks.go`) defining the schema for webhook endpoints, including: - Supported event types (`async_job.completed`, `async_job.failed`) - Per-endpoint delivery tuning knobs (retries, backoff, timeouts, concurrency) - Custom delivery headers stored encrypted at rest - Signing secrets generated in Standard Webhooks format (`whsec_` + base64 of 32 random bytes), encrypted at rest and never included in JSON serialization - `BeforeSave`/`AfterFind` GORM hooks handling serialization of virtual fields (`Events`, `Headers`) and encryption/decryption of sensitive columns - `Validate()` enforcing name, URL (HTTPS required unless `allow_private_network` is set, no credentials or fragments, private/link-local ranges blocked), event list, and header name rules - Added `migrationAddWebhookEndpointsTable` database migration step - Added `ConfigStore` interface methods: `GetWebhookEndpoints`, `GetWebhookEndpointByID`, `GetWebhookEndpointByName`, `CreateWebhookEndpoint`, `UpdateWebhookEndpoint`, `DeleteWebhookEndpoint`, `RotateWebhookEndpointSecret` - Implemented all store methods on `RDBConfigStore`: - `CreateWebhookEndpoint` auto-generates an ID and signing secret when not supplied, enforces name uniqueness, and returns the plaintext secret on the struct for one-time display - `UpdateWebhookEndpoint` resets the consecutive-failure counter only on URL changes; re-enabling a disabled endpoint does not reset it - `RotateWebhookEndpointSecret` replaces the signing secret atomically; only the new secret is stored - Added stub implementations of all new interface methods to `MockConfigStore` in the HTTP transport test package ## 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/... ./framework/configstore/tables/... ./transports/bifrost-http/lib/... ``` Key test scenarios covered: - `TestWebhookEndpointCreate` — ID and `whsec_`-prefixed secret are generated server-side; distinct endpoints receive distinct secrets - `TestWebhookEndpointCreateDuplicateName` — returns `ErrAlreadyExists` - `TestWebhookEndpointUpdate` — non-URL changes preserve the failure counter; URL changes reset it; signing secret is never modified - `TestWebhookEndpointUpdateReenablePreservesFailureCounter` — re-enabling a disabled endpoint does not reset the failure counter - `TestWebhookEndpointRotateSecret` — new secret is stored immediately; old secret is gone - `TestTableWebhookEndpoint_EncryptDecrypt` — ciphertext is stored, plaintext is restored after read - `TestTableWebhookEndpoint_SecretsExcludedFromJSON` — secret material never appears in JSON output - `TestTableWebhookEndpoint_HeadersEncryptDecrypt` — custom headers are encrypted at rest; env references survive the round-trip as references ## Breaking changes - [x] Yes - [ ] No Any type implementing the `ConfigStore` interface must add the seven new webhook endpoint methods. The `MockConfigStore` in the HTTP transport package has been updated; other mocks in the codebase will need the same stub additions. ## Security considerations - Signing secrets are generated using `crypto/rand` and formatted as `whsec_` + base64(32 bytes), following the Standard Webhooks specification. - Secrets and custom delivery headers are encrypted at rest using the existing `encrypt` package when encryption is enabled. - Secrets are excluded from JSON serialization (`json:"-"`) and are surfaced in plaintext only once — at creation or rotation time — via the in-memory struct field. - Webhook URLs are validated against `bifrost.ValidateExternalURL`, which rejects link-local and metadata address ranges regardless of the `allow_private_network` flag. HTTP is only permitted when `allow_private_network` is explicitly set. - Credentials and fragments are rejected from webhook URLs. - Standard Webhooks signing headers and protocol-level headers are protected and cannot be overridden by caller-supplied custom headers. ## 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) - [x] I verified the CI pipeline passes locally if applicable
…ore methods and tests (maximhq#5248) ## Summary Adds the persistence layer for webhook delivery: a `webhook_jobs` work-queue table for in-flight deliveries and a `webhook_deliveries` history table for delivery attempt records, along with the store methods needed to drive the delivery worker loop. ## Changes - **`webhook_jobs` table** (`configstore`): New `TableWebhookJob` model and `add_webhook_jobs_table` migration. Rows exist only while a delivery is pending or retrying and are deleted on terminal outcome. The row ID doubles as the stable `webhook-id` wire header across attempts. - **Webhook job store methods** (`configstore`): `CreateWebhookJob`, `ListDueWebhookJobs`, `ClaimWebhookJob`, `RescheduleWebhookJob`, and `DeleteWebhookJob`. Claiming uses a conditional UPDATE so at most one concurrent worker wins per job; expired leases make rows reclaimable for crash recovery. - **Endpoint delivery counters** (`configstore`): `RecordWebhookEndpointSuccess` and `RecordWebhookEndpointFailure` update `consecutive_failures` and the last-success/failure timestamps via atomic column expressions, bypassing save hooks so config fields and `updated_at` are never touched. - **`webhook_deliveries` table** (`logstore`): New `WebhookDelivery` model, `webhook_deliveries_init` migration, and ClickHouse table creation. Rows are insert-only delivery attempt metadata (no payloads). Expiry is handled by `DeleteExpiredWebhookDeliveries` with batched deletes. - **`async_jobs` schema** (`logstore`): `async_jobs_add_webhook_endpoint_id_column` migration adds `webhook_endpoint_id` to reference the endpoint to notify on job completion. - **`LogStore` and `ConfigStore` interfaces** extended with all new methods; `HybridLogStore` delegates to its inner store. - **`MockConfigStore`** in the HTTP transport test suite updated to satisfy the expanded `ConfigStore` interface. ## 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/... ./framework/logstore/... ``` Key test coverage added: - `TestWebhookEndpointRecordFailureAndSuccess` — counter increment/reset and not-found errors - `TestWebhookEndpointCounterUpdatesLeaveConfigUntouched` — verifies `updated_at` and config fields are not modified - `TestWebhookJobCreateValidation`, `TestWebhookJobCreateDefaultsAndDuplicate` — input validation and duplicate detection - `TestWebhookJobClaimRace`, `TestWebhookJobClaimLeaseExpiryReclaim`, `TestWebhookJobClaimNotDueRejected` — claim fencing and lease expiry recovery - `TestWebhookJobListDue` — ordering, future-job exclusion, live-lease exclusion, expired-lease inclusion - `TestWebhookJobReschedule`, `TestWebhookJobDelete` — ownership fencing on reschedule and delete - `TestWebhookJobClaimCycleWithEmptyRunnerID` — single-node mode with empty runner ID - `TestWebhookDeliveryCreateAndFind`, `TestWebhookDeliverySearchPagination`, `TestWebhookDeliveryDeleteExpired` — delivery history CRUD - `TestLogStoreParity` extended with a `WebhookDeliveries` phase covering all three backends ## Breaking changes - [ ] Yes - [x] No ## Security considerations Webhook secrets are not stored in or exposed by any of the new tables. Delivery counter updates bypass GORM save hooks to prevent accidental secret re-encryption or field clobbering during high-frequency operational writes. ## 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
…ndard Webhooks signing, and per-endpoint retry/concurrency tuning (maximhq#5249) ## Summary This PR introduces the webhook delivery subsystem for Bifrost. It implements the full pipeline for signing, dispatching, and recording webhook notifications when async jobs reach a terminal state, using at-least-once delivery semantics backed by a persistent job queue. ## Changes - **`signer.go`**: Implements Standard Webhooks-compliant HMAC-SHA256 signing (`v1,<base64>` wire format). The signed content covers `{webhookID}.{unix timestamp}.{body}`, matching the reference specification vector. - **`client.go`**: Introduces `deliveryClient`, which maintains two HTTP clients — a strict SSRF-safe client that blocks private/link-local IPs, and a private client for endpoints registered with `allow_private_network` that still blocks unspecified and link-local (e.g. cloud metadata) addresses. Both clients refuse redirects and require TLS ≥ 1.2. Timeouts are fully delegated to the per-attempt context rather than held on the client, allowing safe connection pool sharing. - **`payload.go`**: Renders the JSON delivery envelope (`eventEnvelope`) for live async job rows, with optional response inlining bounded by a per-endpoint payload cap. Provides a degraded `renderExpiredPayload` path for when the async job row has TTL-lapsed before the delivery fires, delivering a `result_expired: true` body instead of dropping the notification. - **`dispatcher.go`**: Implements `Dispatcher`, the queue worker that scans for due jobs, atomically claims them with a lease, performs one delivery attempt per claim, records history, and either reschedules (retryable failures) or retires (success, permanent failure, exhausted budget) the job. Retry backoff is exponential with ±20% jitter, capped at a configurable max. Per-endpoint concurrency is bounded by an in-process slot counter. `EnqueueJobEvent` inserts queue rows at job completion and wakes the worker. `DeliverTest` sends a signed sample event through the full production path without touching the queue or counters. - **Tests**: Full coverage across signing (including the Standard Webhooks reference vector), payload rendering (thin, include-response, oversized, expired), and dispatcher behavior (success, retryable failure, exhausted budget, permanent failure, redirect refusal, SSRF blocking, expired job degraded payload, endpoint-gone retirement without counter movement, claim fencing, lease recovery, test delivery, custom headers, per-endpoint tuning overrides). Notable design decisions: - The claim-and-lease pattern means the dispatcher can run on every node with no coordination; the atomic claim decides a single owner per attempt. - History insert failure intentionally leaves the job claimed so the lease expiry re-offers it, and the receiver's `webhook-id` deduplication absorbs any resulting duplicate delivery. - Reserved delivery headers (`webhook-id`, `webhook-timestamp`, `webhook-signature`, `Content-Type`, `User-Agent`, `X-Bifrost-Event`) always win over endpoint-configured custom headers, even if validation was bypassed. ## 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/webhooks/... ``` The test suite spins up local `httptest` servers and in-memory fakes for the config store, log store, and endpoint resolver. Key scenarios covered: - A successful delivery retires the queue row and increments the endpoint success counter. - A 5xx response reschedules the job with exponential backoff and increments the failure counter. - After the attempt budget is exhausted, the job is retired with outcome `exhausted`. - A 3xx response is treated as a permanent failure (redirects are never followed). - A loopback receiver is unreachable without `allow_private_network`; link-local addresses are blocked even with it. - An expired async job row results in a degraded `result_expired` payload being delivered rather than the notification being dropped. - A job with an expired lease from a dead node is reclaimed and delivered on restart. - Custom endpoint headers are forwarded; reserved header names are silently dropped. ## Breaking changes - [ ] Yes - [x] No ## Security considerations - The strict delivery client uses SSRF-safe dial logic that re-resolves and re-validates IPs at dial time, preventing DNS rebinding attacks where a hostname resolves to a public IP at registration but flips to a private one at delivery. - The private-network client still blocks unspecified (`0.0.0.0`) and link-local (`169.254.x.x`) addresses, preventing access to cloud metadata endpoints even for endpoints explicitly granted private network access. - Signing secrets are base64-decoded at sign time; an empty or malformed secret is rejected before any I/O occurs. - Reserved delivery headers (`webhook-signature`, etc.) are always set by the delivery client and cannot be overridden by endpoint-configured custom headers. - Both clients require TLS ≥ 1.2 and do not follow redirects. ## 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) - [x] I verified the CI pipeline passes locally if applicable
…tate webhook notifications via `BifrostContextKeyAsyncWebhook` (maximhq#5250) ## Summary This PR introduces the plumbing for webhook notifications on async job terminal states. A new `WebhookDispatcher` interface is added to the async job executor, allowing implementations to be notified (via `EnqueueJobEvent`) when a job reaches a completed or failed state. The webhook endpoint ID is carried through the request context (`x-bf-async-webhook`) and stamped onto the job record at submission time. Notifications are only dispatched after the terminal DB write succeeds, ensuring polling callers and webhook receivers see a consistent state. ## Changes - Added `BifrostContextKeyAsyncWebhook` context key to carry the normalized webhook endpoint ID from the submit path into the job executor. - Introduced the `WebhookDispatcher` interface with a single non-blocking `EnqueueJobEvent` method. - Extended `AsyncJobExecutor` to accept an optional `WebhookDispatcher`; a `nil` value disables webhook notifications safely. - `SubmitJob` now reads the webhook endpoint ID from context and stamps it onto the `AsyncJob` record before persisting. - `executeJob` now calls `notifyWebhook` after every successful terminal DB write (completed, failed, or panic-recovered failed). Notifications are skipped if the terminal write itself fails, preventing contradictory state between polling and webhook consumers. - `AsyncJobCleaner` now also reaps expired webhook delivery history records as part of its cleanup cycle. - Updated `NewAsyncJobExecutor` signature to include `WebhookDispatcher`; all call sites updated accordingly (passing `nil` until a concrete dispatcher is wired in). ## 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/logstore/... -v -run TestSubmitJob_StampsWebhookEndpointID go test ./framework/logstore/... -v -run TestSubmitJob_NoWebhookWithoutContextValue go test ./framework/logstore/... -v -run TestExecuteJob_WebhookEnqueuedOnSuccess go test ./framework/logstore/... -v -run TestExecuteJob_WebhookEnqueuedOnFailure go test ./framework/logstore/... -v -run TestExecuteJob_WebhookEnqueuedOnPanic go test ./framework/logstore/... -v -run TestExecuteJob_NilDispatcherIsSafe go test ./framework/logstore/... -v -run TestAsyncJobCleaner_ReapsExpiredWebhookDeliveries go test ./... ``` ## Breaking changes - [x] Yes - [ ] No `NewAsyncJobExecutor` now requires a `WebhookDispatcher` argument (third positional parameter, before `logger`). Any caller constructing an `AsyncJobExecutor` directly must be updated to pass `nil` or a concrete dispatcher. ## Related issues ## Security considerations The webhook endpoint ID stored on the job record is expected to be pre-validated and normalized to an internal endpoint ID by the submit path before the context value is set. Raw caller-supplied endpoint references should never reach the executor directly. ## 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
…nc, and per-endpoint delivery tuning (maximhq#5251) ## Summary This PR wires up the webhook admin API and integrates webhook endpoint configuration into the config.json lifecycle. It adds a `WebhookConfig` global settings struct to the client config, a `webhooks` section to config.json for declaring endpoints declaratively, an in-memory endpoint store on `Config` for zero-database-read serving on the hot path, and a full set of admin HTTP handlers for endpoint CRUD, secret rotation, test delivery, delivery history, and redelivery. ## Changes - Added `WebhookConfig` to `ClientConfig` and `TableClientConfig`, stored as a JSON blob column (`webhook_config_json`) with `BeforeSave`/`AfterFind` serialization. Includes a `DeliveryHistoryRetention()` helper with a 30-day default on a nil receiver. - Added a database migration (`add_webhook_config_client_column`) to introduce the column. - Added `GenerateWebhookEndpointHash` for deterministic change detection of endpoint declarations, covering all declared fields (sorted events and headers for stability) while excluding operational counters and generated IDs. - Added `WebhookEndpointConfig` to `ConfigData` and the `loadWebhooksConfig` startup step, which validates declarations with a warn-and-skip policy and reconciles them against the database using either an additive merge or a full sync depending on whether `source_of_truth: config.json` is set and the `webhooks` section is physically present. - Added an in-memory webhook endpoint store (`webhookEndpoints`, `webhookEndpointsByName`) on `Config`, guarded by a dedicated `muWebhooks` mutex, with `WebhookEndpointByID`, `WebhookEndpointByName`, `SetWebhookEndpoint`, `RemoveWebhookEndpoint`, and `replaceWebhookEndpoints` methods. Handlers keep it in lockstep with every database write. - Added `WebhookHandler` with routes for list, get, create, update, delete, rotate-secret, test delivery (with a per-endpoint 10-second cooldown), delivery history pagination, and redelivery. Custom header values are redacted in all API responses; masked placeholders round-tripped through an update are restored from the in-memory store to avoid corrupting stored values. - Wired `WebhookDispatcher` into `BifrostHTTPServer.Bootstrap`, passing it to the async job executor and stopping it on shutdown. The dispatcher is only initialized when both the config store and logs store are available. - Extended `config.schema.json` with the `webhooks` array and `client.webhook_config` object schemas. Notable design decisions: - The in-memory store deliberately does not mirror operational counters (failure counts, timestamps); list views that need them read the database directly. - Config hash for `WebhookConfig` is only written when the field is non-nil to avoid hash churn on upgrade for existing deployments. - Pruning in source-of-truth mode requires the `webhooks` key to be physically present in the file, preventing an absent section from wiping all database endpoints. - The typed-nil dispatcher problem for the async job executor interface is handled explicitly to avoid a non-nil interface wrapping a nil pointer. ## 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/... ``` - Create a webhook endpoint via `POST /api/webhooks` and verify the secret is returned exactly once. - Rotate the secret via `POST /api/webhooks/{id}/rotate-secret` and confirm the old secret is no longer used for signing. - Add a `webhooks` section to config.json and restart; verify the endpoint appears in the database and is served from memory. - Set `source_of_truth: config.json`, remove an endpoint from the file, and restart; verify the database row is pruned. - Set `source_of_truth: config.json` without a `webhooks` key; verify existing database endpoints are not pruned. - Fire a test delivery via `POST /api/webhooks/{id}/test` and confirm the receiver gets a signed request; fire again immediately and confirm a 429 response. - Seed a failed delivery and redeliver via `POST /api/webhooks/deliveries/{id}/redeliver`; confirm the job appears in the queue under the original webhook ID and a second redeliver returns 409. New config fields: | Field | Location | Description | |---|---|---| | `client.webhook_config.delivery_history_retention_days` | config.json `client` block | How long delivery history rows are kept (default: 30 days) | | `webhooks[].max_retries` | config.json `webhooks` array | Per-endpoint retry count (default: 4) | | `webhooks[].attempt_timeout_seconds` | config.json `webhooks` array | Per-attempt timeout (default: 10s) | | `webhooks[].max_concurrent_deliveries` | config.json `webhooks` array | Concurrency cap per node (default: 10) | ## Breaking changes - [ ] Yes - [x] No ## Security considerations - Signing secrets are generated server-side and returned exactly once at creation or rotation; they are never included in read responses or list views. - Custom header values (e.g. `Authorization`) are encrypted at rest and redacted in all API responses; only header names are visible. - Reserved delivery headers (`webhook-id`, `webhook-timestamp`, `webhook-signature`, `content-type`) cannot be overridden by callers. - Plain HTTP delivery URLs require explicit `allow_private_network: true`; link-local and metadata addresses are always blocked regardless of that flag. - The per-endpoint test delivery cooldown (10 seconds) limits abuse of the test endpoint against external receivers. ## 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) - [ ] I verified the CI pipeline passes locally if applicable
…WebhookManager` instead of pre-normalizing to ID in context (maximhq#5252) ## Summary Webhook endpoint references in async job submissions are now resolved and validated at submit time rather than being pre-normalized to an endpoint ID before the job is created. This ensures that a job referencing an unknown or disabled webhook endpoint is rejected immediately, preventing silent notification loss. ## Changes - Replaced `BifrostContextKeyAsyncWebhook` (which previously expected a pre-resolved endpoint ID) with `BifrostContextKeyAsyncWebhookEndpoint` (which carries the raw endpoint name from the `x-bf-async-webhook` header). - Introduced a `WebhookManager` interface on `AsyncJobExecutor` with a `WebhookEndpointByName` method, used to resolve and validate the endpoint name at submit time. - `SubmitJob` now calls `getWebhookEndpointIfPresent`, which looks up the named endpoint via `WebhookManager`, rejects unknown endpoints, and rejects disabled endpoints — returning an error rather than silently accepting the job. - `NewAsyncJobExecutor` accepts a `WebhookManager` parameter; a nil manager causes any submit that references a webhook endpoint to fail. - The HTTP transport's `ConvertToBifrostContext` now explicitly handles the `x-bf-async-webhook` header, trimming whitespace and storing the value under the new context key. Previously this relied on implicit pass-through and pre-resolution elsewhere. - Removed the now-unnecessary `getWebhookEndpointIDFromContext` helper. - Added tests covering rejection of unknown and disabled endpoint references, and header parsing behavior in the HTTP transport. ## Type of change - [ ] Bug fix - [x] Feature - [x] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./framework/logstore/... ./transports/bifrost-http/lib/... ``` - Submit an async job with `x-bf-async-webhook: <valid-endpoint-name>` — the job should be accepted and the endpoint ID stamped on the job record. - Submit with an unknown endpoint name — expect an error response and no job created. - Submit with a disabled endpoint name — expect an error response and no job created. - Submit without the header — job should be accepted with no webhook endpoint attached. ## Breaking changes - [x] Yes - [ ] No `NewAsyncJobExecutor` now requires a `WebhookManager` argument. Any caller constructing an `AsyncJobExecutor` directly must be updated to pass a `WebhookManager` implementation (or `nil` to disable webhook support). The context key used to carry the webhook reference has changed from `BifrostContextKeyAsyncWebhook` to `BifrostContextKeyAsyncWebhookEndpoint`; any code setting the old key will no longer have effect. ## Related issues ## Security considerations Endpoint resolution is now enforced at job submission, preventing a caller from referencing an arbitrary or disabled webhook endpoint. A nil `WebhookManager` causes any webhook-referencing submit to fail explicitly rather than silently dropping the notification. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [x] 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
…, secret reveal, and sidebar entry (maximhq#5264) ## Summary Adds a full webhook endpoint management UI to the workspace, allowing users to register, configure, and monitor HTTPS endpoints that receive signed notifications when async inference jobs complete or fail. ## Changes - Added a new `/workspace/webhooks` route with RBAC gating via `RbacResource.Governance` — users without view access see a `NoPermissionView` - Built `WebhooksView` as the main table listing all endpoints with name, URL, subscribed events, and an enable/disable toggle; supports search by name or URL - Built `WebhookSheet` (create/edit slide-over) with fields for name, URL, event subscriptions, include-response toggle, private-network toggle, custom headers (via `HeadersTable` with `SecretVar` support), and an expandable delivery tuning accordion covering retries, backoff, timeout, payload size cap, and concurrency - Built `WebhookDetailsSheet` showing endpoint metadata and a paginated delivery history table; deliveries are grouped into runs (original send + redeliveries) with per-run attempt expansion, outcome/status badges with error tooltips, and per-row redeliver actions; polling refreshes the list every 5 seconds - Built `WebhookSecretDialog` to display the signing secret exactly once after create or rotate-secret, with a copy button and a link to the verification docs; the secret is never retrievable again after this dialog is dismissed - Added a `WebhooksEmptyState` shown when no endpoints exist yet - Added `WebhookActionsMenu` with edit, rotate-secret, and delete actions; delete and rotate are confirmed via `AlertDialog`s - Implemented per-endpoint test-fire with a 30-second cooldown tracked in the UI; a 429 from the server resumes the countdown from the server-reported `retry_after_seconds` - Added `webhooksApi` RTK Query endpoints: `getWebhookEndpoints`, `createWebhookEndpoint`, `updateWebhookEndpoint`, `deleteWebhookEndpoint`, `rotateWebhookEndpointSecret`, `testWebhookEndpoint`, `getWebhookDeliveries`, and `redeliverWebhookDelivery`; registered `WebhookEndpoints` and `WebhookDeliveries` cache tags in `baseApi` - Added `webhooks.ts` type definitions covering `WebhookEndpoint`, `WebhookEndpointRequest`, `WebhookDelivery`, `WebhookDeliveryOutcome`, response shapes, event color maps, tuning defaults, and the test cooldown constant - Added a "Webhooks" entry to the sidebar under the Governance section ## 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 build || npm run build ``` 1. Navigate to **Workspace → Webhooks** with a user that has Governance view access. 2. Click **Add Webhook Endpoint**, fill in a name and an HTTPS URL, select at least one event, and save — the signing secret dialog should appear exactly once. 3. From the endpoint row's actions menu, click **Rotate secret** and confirm — the new secret dialog should appear. 4. Click a row to open the details sheet; submit an async job referencing the endpoint and verify deliveries appear with correct outcome badges. 5. Use **Send Test Event** from the details sheet and confirm the 30-second cooldown activates on both the sheet button and the table row. 6. Expand a delivery run with multiple attempts and verify per-attempt rows show attempt number, outcome, and status code. 7. Click the redeliver button on a delivery and confirm a new run appears. 8. Delete an endpoint and confirm pending deliveries are dropped and the row is removed. 9. Log in as a user without Governance access and confirm the route renders `NoPermissionView`. ## Screenshots/Recordings _Add before/after screenshots or clips of the webhooks table, create sheet, secret dialog, and details sheet._ ## Breaking changes - [ ] Yes - [x] No ## Related issues _Link related issues here._ ## Security considerations - The signing secret is returned by the API exactly once (on create and rotate) and is never stored or re-exposed by the UI after the dialog is dismissed. - Custom header values are stored as `SecretVar` and arrive fully redacted from the server; the UI round-trips placeholders as-is so the server restores the real credentials without them transiting the browser. - The `allow_private_network` toggle is required to be enabled before an `http://` URL is accepted, preventing accidental SSRF to private ranges. - Test fires are rate-limited server-side with a 30-second cooldown; the UI enforces the same limit locally and resumes from the server-reported `retry_after_seconds` on a 429. ## 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
… deliveries for LLM log reconciliation (maximhq#5265) ## Summary Async jobs and webhook deliveries now carry the originating inference request ID, making it possible to correlate a webhook delivery history record directly with its LLM log entry without having to join through the async job. ## Changes - Added `request_id` field to `AsyncJobResponse`, `AsyncJob` (DB table), and `WebhookDelivery` (DB table). - When a job is submitted, the request ID is read from the `BifrostContext` and stored on the `AsyncJob` row. - When the webhook dispatcher attempts a delivery, it copies `request_id` from the resolved async job onto the `WebhookDelivery` history record. If the job row has already expired, the field is left empty. - Two new database migrations add the `request_id` column to `async_jobs` and `webhook_deliveries` for existing databases. - The webhook details UI now displays `request_id` (truncated, copyable) instead of `async_job_id` in the delivery history table. A `-` placeholder is shown when the field is absent (e.g. deliveries recorded before the migration). - The `WebhookDelivery` TypeScript type gains an optional `request_id` field. - A new test (`TestSubmitJob_StoresRequestID`) verifies that the request ID is persisted on both the in-memory job struct and the stored row. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh # Core/Transports go version go test ./... # UI cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` Submit an async request with a traceable request ID, then open the webhook details sheet for the corresponding endpoint. The delivery history row should display the truncated request ID (matching the LLM log entry) rather than the async job ID. For deliveries recorded before the migration, a `-` should appear. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations `request_id` values are internal correlation identifiers and do not contain secrets or PII. They are already surfaced in LLM log rows; exposing them in async job and webhook delivery records does not introduce new information disclosure. ## 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
…dpoints list (maximhq#5266) ## Summary The webhook endpoints list API and UI previously fetched all records at once and filtered client-side. This PR moves filtering and pagination to the server, enabling efficient handling of large endpoint sets. ## Changes - Added `GetWebhookEndpointsPaginated` to the `ConfigStore` interface and its RDB implementation, supporting search (name/URL, case-insensitive), event subscription filtering (OR semantics via JSON substring match), disabled/enabled state filtering, and limit/offset pagination with a total count returned alongside the page. - Updated `listWebhookEndpoints` in the HTTP handler to parse and validate `search`, `event`, `disabled`, `limit`, and `offset` query parameters, delegating all filtering to the new paginated store method. The response envelope now includes `total_count`, `limit`, and `offset` alongside the existing `count` and `endpoints` fields. - Replaced the client-side search filter in `WebhooksView` with server-driven query parameters persisted in the URL via `nuqs`. Search input is debounced before triggering a fetch. A `WebhooksFilterBar` component was extracted to house the search input, event multi-select, and status multi-select, with a "Clear filters" button shown when any filter is active. - Added a pagination footer to the webhooks table with previous/next controls and a "Page X of Y" indicator. The offset auto-corrects to the last valid page when the total count drops below the current position (e.g., after a delete or filter narrowing). - Added `GetWebhookEndpointsParams` and extended `GetWebhookEndpointsResponse` in the frontend type definitions. The RTK Query endpoint now accepts optional params and serializes them canonically (events sorted and CSV-joined) for stable cache keys. - Added unit tests for `GetWebhookEndpointsPaginated` covering no-filter, paging, search, disabled, event, and composed filter cases. Added HTTP handler tests covering all filter parameters and invalid input rejection. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh # Core/Transports go test ./framework/configstore/... ./transports/bifrost-http/... # UI cd ui pnpm i pnpm test pnpm build ``` Navigate to the Webhooks page with several endpoints configured. Verify: - Search by name or URL filters results in real time (after the 300 ms debounce). - The event and status dropdowns filter independently and compose with search. - "Clear filters" resets all filters and returns to page 1. - Pagination controls advance and retreat through pages; the offset resets to 0 when a filter changes. - Filter state survives a page refresh (URL query params are preserved). - Invalid `disabled`, `event`, `limit`, and `offset` values return HTTP 400. ## Screenshots/Recordings _Add before/after screenshots of the webhooks table with the new filter bar and pagination footer._ ## Breaking changes - [x] Yes - [ ] No The `GET /webhooks` response envelope now includes `total_count`, `limit`, and `offset` fields. Existing consumers that only read `endpoints` and `count` are unaffected, but consumers expecting the full list in a single unfiltered call should be aware that results are now paginated (default page size 25). ## Related issues ## Security considerations Event filtering uses parameterized LIKE queries against a JSON string column; no raw user input is interpolated into SQL. The `disabled` parameter is parsed as a strict boolean before use. No secrets or PII are introduced. ## 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
…irtualKeys` (maximhq#5313) ## Summary Fixes the visibility condition for the `ApiKeySelectorView` component so it correctly shows when a provider is selected, displaying API key options based on whether virtual keys are enforced. ## Changes - When `enforceVirtualKeys` is true, the `ApiKeySelectorView` now only renders if there are virtual keys available for the selected provider - When `enforceVirtualKeys` is false, the component renders for any selected provider regardless of whether provider keys exist - The previous logic incorrectly hid the selector when no non-virtual provider keys were present but virtual keys were not enforced, and required provider keys to exist even when they weren't needed ## Type of change - [x] Bug fix ## Affected areas - [x] UI (React) ## How to test 1. Select a provider in the settings panel 2. With `enforceVirtualKeys` disabled, verify the `ApiKeySelectorView` appears regardless of whether provider keys are configured 3. With `enforceVirtualKeys` enabled, verify the `ApiKeySelectorView` only appears when virtual keys exist for the selected provider 4. Verify no selector appears when no provider is selected ```sh cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` ## Breaking changes - [x] No ## Security considerations No security implications. This change only affects UI rendering logic for API key selection. ## 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
…error_details` from list query (maximhq#5327) ## Summary This PR fixes a Cloud Run 500 error caused by the `/api/logs` response exceeding the 32MB body limit when many error-status log rows are returned, and replaces the auto-calculated table page size with a user-controlled, localStorage-persisted page size preference. ## Changes - **Exclude `error_details` from the logs list query**: The `error_details` column is dropped from `listSelectColumns` in `rdb.go` because it can carry unbounded provider error payloads. With 25+ such rows, the combined response body can exceed Cloud Run's 32MB limit and return a 500. The full error detail remains available via the individual log detail endpoint (`GET /api/logs/{id}`). - **Replace auto-sized page size with a user preference**: The `useTablePageSize` hook (which inferred page size from container height) is replaced by `useTablePageSizePreference`, which stores the user's chosen page size in localStorage under a per-table key (e.g. `bifrost.logs.pageSize`). The preference is hydrated lazily after mount to avoid SSR/hydration mismatches and defaults to 25. - **Add a "Rows per page" dropdown to the logs table footer**: A `ComboboxSelect` with options `[10, 25, 50, 100, 200]` is added to the pagination controls, allowing users to explicitly choose how many rows to load per page. The selection is persisted across sessions. ## Type of change - [x] Bug fix - [x] Feature ## Affected areas - [x] Core (Go) - [x] UI (React) ## How to test 1. Navigate to the logs table in the UI. 2. Confirm a "Rows per page" dropdown appears in the pagination footer with options 10, 25, 50, 100, and 200. 3. Select a page size, reload the page, and verify the selection is restored from localStorage. 4. Confirm that log list responses no longer include `error_details` in the payload, and that the full error is still visible when opening an individual log entry. 5. Verify that workspaces with many error-status logs no longer trigger 500 responses from `/api/logs`. ```sh # Core go test ./framework/logstore/... # UI cd ui pnpm i pnpm test pnpm build ``` ## Screenshots/Recordings Before: Page size was inferred from container height with no user control. After: A "Rows per page" dropdown is shown in the pagination bar, and the selection persists across page reloads. ## Breaking changes - [x] No The `error_details` field is removed from list responses but remains available on the detail endpoint. Clients relying on `error_details` in the list response will need to fetch individual log entries to retrieve it. ## Related issues Closes the Cloud Run 32MB body limit 500 error on `/api/logs`. ## Security considerations No new auth, secrets, or PII exposure. Removing `error_details` from the list response reduces the amount of potentially sensitive provider error data sent in bulk responses. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
## Summary Documents the new MCP (Model Context Protocol) tool call observability support across Prometheus, OpenTelemetry, and Datadog integrations. This exposes `bifrost_mcp_client_operation_duration_seconds` (and its OTel/Datadog equivalents) so operators can monitor MCP tool call volume, latency, and failures with full governance identity context. ## Changes - Added `bifrost_mcp_client_operation_duration_seconds` histogram to the Prometheus metrics reference, including label definitions (`mcp_client`, `mcp_tool_name`, `mcp_method`, `error_type`, and governance labels) and a note clarifying that only tool executions are recorded (lifecycle ops and codemode tools are skipped) - Added MCP tool call span documentation to the OTel plugin, describing the semantic-convention attributes (`mcp.method.name`, `gen_ai.tool.name`, `network.transport`, `error.type`) and the `mcp.client.operation.duration` OTLP metric export - Added `bifrost.mcp.client.operation.duration` histogram to the Datadog connector metrics table with its tag dimensions - Added a dedicated MCP Metrics section to the telemetry reference with a full label breakdown and a callout noting the differences from LLM metrics (no `provider`/`model` or `network_transport` labels) ## 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 pages for: - `docs/features/telemetry.mdx` — MCP Metrics section appears with correct table and labels - `docs/features/observability/prometheus.mdx` — Bifrost MCP Metrics section appears under the metrics reference - `docs/features/observability/otel.mdx` — MCP tool call spans and `mcp.client.operation.duration` metric are described - `docs/enterprise/datadog-connector.mdx` — `bifrost.mcp.client.operation.duration` row appears in the metrics table ## Breaking changes - [ ] Yes - [x] No ## Related issues ## 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
## Summary Adds `user_email` as a tracked attribute in Bifrost tracing and enrichment, alongside the existing `user_id` and `user_name` fields. This allows user email to be captured from the enterprise auth middleware context and propagated through spans and BigQuery enrichment dimensions. ## Changes - Added `BifrostContextKeyUserEmail` context key (`"bifrost-user-email"`) for storing user email set by the enterprise auth middleware - Added `AttrBifrostUserEmail` span attribute constant (`"bifrost.user.email"`) - Added `user_email` to `EnrichmentDims` for BigQuery column and span attribute propagation - Extended `PopulateContextAttributes` to accept and emit `userEmail` alongside `userID` and `userName` - Added span attribute setting in `executeRequestWithRetries` when a user email is present in context ## 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 Verify that spans emitted during request execution include `bifrost.user.email` when a user email is present in the context, and that the enrichment pipeline maps it to the `user_email` BigQuery column. ```sh go test ./... ``` ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations User email is PII. It is sourced exclusively from the enterprise auth middleware context and should not be set manually. Ensure downstream span exporters and BigQuery destinations handling `user_email` apply appropriate access controls consistent with those already in place for `user_id` and `user_name`. ## 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
…ic surface OpenAI validates encrypted reasoning payloads against the reasoning item id they were issued with. The Anthropic surface drops that id on egress (the redacted_thinking block only carries type and data) and mints a random one on replay, so every replayed redacted block fails with "Encrypted content item_id did not match the target item id" and traffic silently degrades to fallbacks. Carry the pair through the round trip in a Bifrost-private envelope inside data, decoded only at the final destination-provider conversion: wrap on OpenAI-origin egress, restore the id at replay while keeping the envelope in the neutral request, unwrap in ToOpenAIResponsesRequest only when the destination is exactly OpenAI, and drop the hidden item for any other destination instead of forwarding foreign ciphertext. Anthropic-origin data stays byte-identical; unrecognized data keeps the previous behavior. Items carrying both a summary and encrypted content now emit thinking blocks and the redacted block instead of losing the encrypted state. Fixes maximhq#5186
abhishekgahlot2
force-pushed
the
fix/anthropic-openai-encrypted-reasoning-item-id
branch
from
July 17, 2026 18:21
55e667c to
894c3a7
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
core/providers/anthropic/reasoningstream_test.go (1)
108-118: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
schemas.Ptr()over custom closures or the address operator (&).Based on learnings, prefer using
schemas.Ptr()(orbifrost.Ptr()) to create pointers instead of using the address operator (&), even in test utilities, to improve consistency and readability.schemas.Ptr()is already used elsewhere in this file.
core/providers/anthropic/reasoningstream_test.go#L108-L118: replace the localreasoningvariable and&reasoningwith an inlineschemas.Ptr("The user asked how to run core tests.").core/providers/anthropic/reasoningstream_test.go#L39-L47: remove the customphelper and useschemas.Ptr()directly.core/providers/anthropic/reasoningstream_test.go#L143-L149: remove the customphelper and useschemas.Ptr()directly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/providers/anthropic/reasoningstream_test.go` around lines 108 - 118, Replace the local reasoning variable and address-of use in TestConvertBifrostReasoning_SignaturePresent with an inline schemas.Ptr value. In core/providers/anthropic/reasoningstream_test.go lines 39-47 and 143-149, remove the custom p helper and use schemas.Ptr() directly at each call site.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@core/providers/anthropic/reasoningstream_test.go`:
- Around line 108-118: Replace the local reasoning variable and address-of use
in TestConvertBifrostReasoning_SignaturePresent with an inline schemas.Ptr
value. In core/providers/anthropic/reasoningstream_test.go lines 39-47 and
143-149, remove the custom p helper and use schemas.Ptr() directly at each call
site.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 64cc8146-3b91-4ee8-9b72-b5df9d83ac2c
📒 Files selected for processing (7)
core/providers/anthropic/reasoningstream_test.gocore/providers/anthropic/redactedthinkingenvelope_test.gocore/providers/anthropic/responses.gocore/providers/openai/reasoningenvelope_test.gocore/providers/openai/responses.gocore/providers/utils/reasoningenvelope.gocore/providers/utils/reasoningenvelope_test.go
🚧 Files skipped from review as they are similar to previous changes (6)
- core/providers/openai/reasoningenvelope_test.go
- core/providers/utils/reasoningenvelope.go
- core/providers/openai/responses.go
- core/providers/utils/reasoningenvelope_test.go
- core/providers/anthropic/redactedthinkingenvelope_test.go
- core/providers/anthropic/responses.go
akshaydeo
dismissed
coderabbitai[bot]’s stale review
July 17, 2026 23:27
The merge-base changed after approval.
2 tasks
Contributor
|
Hi @abhishekgahlot2 — thanks for the contribution! This PR is currently blocked because our CLA bot shows the Contributor License Agreement as not yet signed. Could you sign it here so we can move this forward: https://cla-assistant.io/maximhq/bifrost?pullRequest=5192 Let us know if you run into any issues signing. |
Author
|
heya @akshaydeo i signed it let me know if this needs rebasing as well. |
This was referenced Aug 4, 2026
Merged
Merged
akshaydeo
force-pushed
the
dev
branch
3 times, most recently
from
August 13, 2026 09:47
244a01d to
ce1b2a6
Compare
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
11 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #5186. OpenAI validates encrypted reasoning payloads against the reasoning item id they were issued with. The Anthropic surface drops that id on egress (the
redacted_thinkingblock only carriestypeanddata) and mints a random one on replay, so every replayed redacted block fails withEncrypted content item_id did not match the target item idand traffic silently degrades to fallbacks. This PR carries the(item_id, encrypted_content)pair through the round trip in a Bifrost-private envelope insidedata, decoded only at the final destination-provider conversion.Changes
providers/utils/reasoningenvelope.go: base64 JSON envelope{_bifrost, v, provider, item_id, payload}. Strict unwrap: exact magic, version, provider allowlist, non-empty id and payload, encoded-size cap, control-character check on the id. Anything that does not parse is raw legacy data and behaves exactly as before. Wrap is idempotent and falls back to the raw payload if the encoded envelope would exceed the cap.ExtraFields.Provider, not the requested model, since routing and fallbacks can change it) and the item has both an id and encrypted content, the redacted block'sdataholds the envelope. Anthropic-origindatastays byte-identical. Items with both a summary and encrypted content now emit the thinking blocks and the redacted block instead of dropping the encrypted state.ResponsesMessageid and stays wrapped in the neutralEncryptedContent. Deferred decoding keeps provenance intact for fallbacks.ToOpenAIResponsesRequestunwraps only when the destination provider is exactly OpenAI. Any other destination through the shared converter (Azure, OpenRouter, fallback-swapped requests) drops the hidden item rather than forwarding foreign ciphertext.Type of change
Affected areas
How to test
Live validation: built
bifrost-httpfrom this branch, captured aredacted_thinkingblock from a GPT-5.x reasoning model through/anthropic/v1/messages(thedatafield now carries the envelope), then replayed it verbatim in a tool loop. On v1.6.3 that replay fails with the item-id 400 every time; on this branch it succeeds. Anthropic-origin thinking replay through the same build is unchanged.