fixed seed placements - #4049
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughExtracts e2e seeding into a standalone cmd/e2eseed Go module and rewires entrypoints/tests to use the seed package, replaces bifrost.Ptr calls with new() in governance virtual-key extraction, adds a dashboard dev-mode proxy to the Vite dev server, and introduces an agent handover route and UI component. ChangesE2E Seed Module Reorganization
Governance Virtual Key HTTP Extraction
Dashboard Dev Proxy and Agent Handover UI
🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 4❌ Failed checks (3 warnings, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
akshaydeo
marked this pull request as ready for review
June 4, 2026 09:24
|
|
Contributor
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Contributor
Confidence Score: 5/5Safe to merge; all changes are additive tooling, a dev-only proxy, and a new UI route with no impact on production request paths. The governance plugin change is a no-op behavioural difference (Go 1.26 new(expr) is equivalent to the previous bifrost.Ptr). The UIHandler dev proxy is correctly guarded behind IsDevMode() and attached as a struct field rather than a global. The agent handover page is an isolated new route. The only concern is the duplicated seed.go between cmd/e2eseed/ and tests/cmd/, which is a maintenance concern but does not affect runtime correctness. cmd/e2eseed/seed/seed.go and tests/cmd/seed/seed.go are exact duplicates; any future logic change must be applied to both files. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Incoming HTTP request] --> B{IsDevMode?}
B -- No --> E[serveDashboard: embedded FS]
B -- Yes --> C{uiDevClient != nil?}
C -- No --> E
C -- Yes --> D[serveDevDashboard: proxy to Vite :3000]
D -- success --> F[Copy resp to ctx.Response]
D -- error / timeout --> E
E --> G[Serve embedded UI asset or SPA fallback]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[Incoming HTTP request] --> B{IsDevMode?}
B -- No --> E[serveDashboard: embedded FS]
B -- Yes --> C{uiDevClient != nil?}
C -- No --> E
C -- Yes --> D[serveDevDashboard: proxy to Vite :3000]
D -- success --> F[Copy resp to ctx.Response]
D -- error / timeout --> E
E --> G[Serve embedded UI asset or SPA fallback]
Reviews (13): Last reviewed commit: "fixed seed placements" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/e2eseed/seed/seed.go`:
- Around line 1-2: Update the top-of-file package comment to match the declared
package name by changing the package docstring from "Package e2eseed ..." to
"Package seed ..." so the package comment and the declaration (package seed) are
consistent; edit the file-level comment at the top of seed.go where the current
package comment appears to reflect the new package name.
In `@plugins/governance/store.go`:
- Around line 27-29: The code updates gs.virtualKeys in many VK rewrite paths
but doesn't update gs.virtualKeysByID, causing GetVirtualKeyByID to become
stale; fix by centralizing VK writes through the existing storeVirtualKey(...)
helper (which should atomically write both gs.virtualKeys and
gs.virtualKeysByID) and replace all direct writes to gs.virtualKeys in rewrite
sites (e.g., team/customer unlink handlers, budget/rate-limit reference refresh
paths, and the locations mentioned in the review) with calls to
storeVirtualKey(VK) so both maps stay synchronized and race-safe for
GetVirtualKey and GetVirtualKeyByID lookups.
In `@transports/bifrost-http/handlers/ui.go`:
- Around line 155-157: The handler currently swallows errors from
uiDevClient.Do(&req, &resp) and silently falls back to embedded files; update
the error path to log the proxy error before returning false. Locate the call to
uiDevClient.Do in the UI request handler and add a log statement that includes
the error (and context like "Dev server proxy failed, falling back to embedded
UI"), using the existing logger in the handler context if available (or the
package logger) so developers can see why the dev-mode proxy failed; keep the
existing return false behavior after logging.
- Around line 16-18: The uiDevClient HostClient is created without timeouts
causing potential indefinite hangs; update the uiDevClient initialization (the
variable uiDevClient that uses uiDevServerAddr) to set ReadTimeout and
WriteTimeout (and optionally IdleTimeout/MaxIdleConnDuration) to reasonable
values (e.g., a few seconds) and add the time import to the file so the
durations compile; ensure the updated HostClient literal includes those timeout
fields to avoid blocking on OS-level TCP timeouts.
In `@ui/app/_fallbacks/enterprise/components/agent/handoverView.tsx`:
- Around line 1-22: The file is named handoverView.tsx but exports a PascalCase
React component AgentHandoverView; rename the file from handoverView.tsx to
AgentHandoverView.tsx and update any imports that reference the old filename to
import AgentHandoverView (preserving the exported component name) so the
filename matches the component (look for usages/imports of AgentHandoverView or
handoverView and update them accordingly).
- Around line 5-6: The component currently memoizes status via useMemo(() => new
URLSearchParams(window.location.search).get("status"), []) which never updates;
remove useMemo and derive status on each render (e.g., const status = new
URLSearchParams(window.location.search).get("status") or read from the router's
search params) so status and the dependent isComplete (const isComplete =
!status || status === "complete") update when the URL changes; update any tests
or usages expecting memoization accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: bbc693b0-795e-4702-b883-f9700367d8e4
⛔ Files ignored due to path filters (1)
cmd/e2eseed/go.sumis excluded by!**/*.sum
📒 Files selected for processing (10)
cmd/e2eseed/go.modcmd/e2eseed/main.gocmd/e2eseed/seed/seed.goplugins/governance/store.goplugins/governance/store_test.goplugins/governance/utils.gotransports/bifrost-http/handlers/ui.goui/app/_fallbacks/enterprise/components/agent/handoverView.tsxui/app/agent/handover/layout.tsxui/app/agent/handover/page.tsx
akshaydeo
force-pushed
the
06-04-fixed_seed_placements
branch
from
June 4, 2026 15:34
035662d to
b39bfd3
Compare
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
core/providers/utils/dialer_test.go (1)
295-392: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAdd coverage for the two untested branches of the new dialer contract.
These tests cover the nil-
Dialpath and proxy bypass, but notallowPrivateNetwork=trueor a client withDialTimeoutset. Both are easy regression points for the new signature, and the latter follows different logic from the path under test today.🤖 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/utils/dialer_test.go` around lines 295 - 392, Add two new tests to cover the missing dialer branches: one that calls ConfigureDialer(client, true) (allowPrivateNetwork=true) and asserts that dialing a private IP (e.g., "10.0.0.1:80") is allowed (no "private IP" SSRF error) and follows normal Dial behavior, and another that constructs a client with DialTimeout set (e.g., client.DialTimeout > 0), calls ConfigureDialer(client, false) and verifies the path that uses timeouts (e.g., that dialing an unroutable address yields a timeout/dial error rather than the proxy/no-op branch). Reference ConfigureDialer, the allowPrivateNetwork parameter, and the client's DialTimeout/Dial field to locate where to add these tests.core/providers/utils/utils.go (2)
278-327:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep the IP-class SSRF checks on the
DialTimeoutpath.This branch dials
addrdirectly. If a caller setsclient.DialTimeoutto customize connect timeouts without also installing a proxy/customDial, the new private/link-local/unspecified IP checks are skipped entirely.🤖 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/utils/utils.go` around lines 278 - 327, The DialTimeout branch (case existingDialTimeout != nil) currently dials addr directly and skips the new SSRF IP-class checks; change it to resolve host -> enforce the same checks (ip.IsUnspecified, network.IsLinkLocal(ip), and block private IPs unless ip.IsLoopback() or allowPrivateNetwork) for each resolved IP, and then call existingDialTimeout using the IP literal + port (preserving client.DialTimeout semantics) or try next resolved IP on failure; reference existingDialTimeout, client.DialTimeout, allowPrivateNetwork and reuse the same validation logic and error messages used in the default branch before dialing.
1090-1117:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRequire a full SSE field token before treating the stream as SSE.
peekHasPrefixcurrently returns true for partial matches. On a chunked first read, a non-SSE body that happens to start withd,e,i, orris misclassified as SSE and bypasses the non-SSE drain.🛠️ Proposed fix
func peekHasPrefix(reader *bufio.Reader, prefix []byte) bool { - n := min(reader.Buffered(), len(prefix)) - if n == 0 { - return false - } - peeked, err := reader.Peek(n) - return err == nil && bytes.Equal(peeked, prefix[:n]) + peeked, err := reader.Peek(len(prefix)) + if err != nil { + return false + } + return bytes.Equal(peeked, prefix) }🤖 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/utils/utils.go` around lines 1090 - 1117, The SSE detection incorrectly accepts partial token matches because peekHasPrefix uses min(reader.Buffered(), len(prefix)) and returns true on partial matches; update peekHasPrefix (used by hasSSEPrefix) to require the full prefix length: if reader.Buffered() < len(prefix) return false, then call reader.Peek(len(prefix)) and only return true when err==nil and bytes.Equal(peeked, prefix). This ensures only complete SSE field tokens ("data:", "event:", "id:", "retry:") are treated as SSE.core/providers/openai/openai.go (1)
6939-6948:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse the OpenAI URL builder for passthrough routes.
Lines 6945 and 7020 still hand-build
BaseURL + "/v1" + path. That bypasses the OpenAI-specific path normalization used for ChatGPTOAuth, so passthrough and passthrough streaming can hit the wrong upstream route when OAuth mode is enabled.♻️ Proposed fix
- url := provider.networkConfig.BaseURL + "/v1" + path + url := provider.buildFullURL("/v1" + path) if req.RawQuery != "" { url += "?" + req.RawQuery }- url := provider.networkConfig.BaseURL + "/v1" + path + url := provider.buildFullURL("/v1" + path) if req.RawQuery != "" { url += "?" + req.RawQuery }Based on learnings: In
core/providers/openai, when ChatGPTOAuth is enabled, Passthrough and PassthroughStream must build"/v1/..."routes throughOpenAIProvider.buildRequestURL(...)orOpenAIProvider.buildFullURL(...)so the/v1normalization logic runs.Also applies to: 7016-7023
🤖 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/openai/openai.go` around lines 6939 - 6948, The code is manually composing URLs with provider.networkConfig.BaseURL + "/v1" + path which bypasses OpenAI-specific normalization (breaking ChatGPTOAuth passthrough); replace these manual concatenations in the passthrough and passthrough-stream handlers by calling the OpenAI provider URL helpers (e.g. OpenAIProvider.buildRequestURL(...) or OpenAIProvider.buildFullURL(...)) so the "/v1" normalization and OAuth path adjustments run; locate usages around where `path` and `req.RawQuery` are combined and swap them to use provider.buildRequestURL(path, req.RawQuery) or the appropriate provider.buildFullURL(req) method on the OpenAIProvider instance.
🤖 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.
Outside diff comments:
In `@core/providers/openai/openai.go`:
- Around line 6939-6948: The code is manually composing URLs with
provider.networkConfig.BaseURL + "/v1" + path which bypasses OpenAI-specific
normalization (breaking ChatGPTOAuth passthrough); replace these manual
concatenations in the passthrough and passthrough-stream handlers by calling the
OpenAI provider URL helpers (e.g. OpenAIProvider.buildRequestURL(...) or
OpenAIProvider.buildFullURL(...)) so the "/v1" normalization and OAuth path
adjustments run; locate usages around where `path` and `req.RawQuery` are
combined and swap them to use provider.buildRequestURL(path, req.RawQuery) or
the appropriate provider.buildFullURL(req) method on the OpenAIProvider
instance.
In `@core/providers/utils/dialer_test.go`:
- Around line 295-392: Add two new tests to cover the missing dialer branches:
one that calls ConfigureDialer(client, true) (allowPrivateNetwork=true) and
asserts that dialing a private IP (e.g., "10.0.0.1:80") is allowed (no "private
IP" SSRF error) and follows normal Dial behavior, and another that constructs a
client with DialTimeout set (e.g., client.DialTimeout > 0), calls
ConfigureDialer(client, false) and verifies the path that uses timeouts (e.g.,
that dialing an unroutable address yields a timeout/dial error rather than the
proxy/no-op branch). Reference ConfigureDialer, the allowPrivateNetwork
parameter, and the client's DialTimeout/Dial field to locate where to add these
tests.
In `@core/providers/utils/utils.go`:
- Around line 278-327: The DialTimeout branch (case existingDialTimeout != nil)
currently dials addr directly and skips the new SSRF IP-class checks; change it
to resolve host -> enforce the same checks (ip.IsUnspecified,
network.IsLinkLocal(ip), and block private IPs unless ip.IsLoopback() or
allowPrivateNetwork) for each resolved IP, and then call existingDialTimeout
using the IP literal + port (preserving client.DialTimeout semantics) or try
next resolved IP on failure; reference existingDialTimeout, client.DialTimeout,
allowPrivateNetwork and reuse the same validation logic and error messages used
in the default branch before dialing.
- Around line 1090-1117: The SSE detection incorrectly accepts partial token
matches because peekHasPrefix uses min(reader.Buffered(), len(prefix)) and
returns true on partial matches; update peekHasPrefix (used by hasSSEPrefix) to
require the full prefix length: if reader.Buffered() < len(prefix) return false,
then call reader.Peek(len(prefix)) and only return true when err==nil and
bytes.Equal(peeked, prefix). This ensures only complete SSE field tokens
("data:", "event:", "id:", "retry:") are treated as SSE.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a265bb53-a1cd-4ffb-bfcd-98e710bcb38f
⛔ Files ignored due to path filters (1)
cmd/e2eseed/go.sumis excluded by!**/*.sum
📒 Files selected for processing (23)
cmd/e2eseed/go.modcmd/e2eseed/main.gocmd/e2eseed/seed/seed.gocore/providers/anthropic/anthropic.gocore/providers/azure/azure.gocore/providers/gemini/gemini.gocore/providers/gemini/gemini_test.gocore/providers/gemini/types.gocore/providers/openai/openai.gocore/providers/utils/dialer_test.gocore/providers/utils/utils.gocore/providers/utils/utils_test.gocore/providers/vertex/vertex.goframework/modelcatalog/config.goframework/modelcatalog/sync.goplugins/governance/store.goplugins/governance/store_test.goplugins/governance/utils.gotests/e2e/api/collections/provider-harness.jsontransports/bifrost-http/handlers/ui.goui/app/_fallbacks/enterprise/components/agent/handoverView.tsxui/app/agent/handover/layout.tsxui/app/agent/handover/page.tsx
akshaydeo
force-pushed
the
06-04-fixed_seed_placements
branch
2 times, most recently
from
June 4, 2026 18:40
1c091f6 to
e148b58
Compare
18 tasks
akshaydeo
force-pushed
the
06-04-fixed_seed_placements
branch
from
June 7, 2026 21:24
e148b58 to
c99bdb2
Compare
18 tasks
akshaydeo
force-pushed
the
06-04-fixed_seed_placements
branch
2 times, most recently
from
June 9, 2026 06:40
22efdd6 to
ed77ca6
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
ui/app/_fallbacks/enterprise/components/agent/handoverView.tsx (2)
1-22: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winRename file to PascalCase to match UI component-file convention.
handoverView.tsxshould be PascalCase (for example,AgentHandoverView.tsx) to align with repo UI rules.🤖 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 `@ui/app/_fallbacks/enterprise/components/agent/handoverView.tsx` around lines 1 - 22, Rename the component file from handoverView.tsx to PascalCase AgentHandoverView.tsx; keep the exported React component name AgentHandoverView as-is, update all imports/usages that reference the old filename (search for "handoverView" imports and update them to "AgentHandoverView"), and ensure any route/fallback registrations or dynamic imports that reference the filename are updated so the bundler/resolver picks up the renamed file.Source: Coding guidelines
5-6:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
statusbecomes stale due to empty-dependency memoization.With
useMemo(..., []), URL search updates on the same mounted route won’t refreshstatus/isComplete.Suggested fix
-import { useMemo } from "react"; ... -const status = useMemo(() => new URLSearchParams(window.location.search).get("status"), []); +const status = new URLSearchParams(window.location.search).get("status");🤖 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 `@ui/app/_fallbacks/enterprise/components/agent/handoverView.tsx` around lines 5 - 6, The current useMemo with an empty dependency causes status/isComplete to become stale; replace the useMemo-based read of window.location.search with a reactive search params hook (e.g., useSearchParams from next/navigation) or otherwise re-evaluate on param change: stop using useMemo(..., []) and instead call const searchParams = useSearchParams(); const status = searchParams.get("status"); const isComplete = !status || status === "complete"; (referencing the existing status and isComplete identifiers and removing the useMemo usage).transports/bifrost-http/handlers/ui.go (2)
16-19:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd explicit timeouts to
uiDevClientto avoid hanging dev requests.
fasthttp.HostClienthere has no read/write timeout, so a stalled local dev server can block dashboard requests for a long time.Suggested fix
+import "time" ... -var uiDevClient = &fasthttp.HostClient{Addr: uiDevServerAddr} +var uiDevClient = &fasthttp.HostClient{ + Addr: uiDevServerAddr, + ReadTimeout: 5 * time.Second, + WriteTimeout: 5 * time.Second, +}🤖 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 `@transports/bifrost-http/handlers/ui.go` around lines 16 - 19, uiDevClient currently constructs a fasthttp.HostClient with no timeouts, which can hang requests; update the uiDevClient creation to set sensible timeouts (e.g., ReadTimeout and WriteTimeout, and optionally IdleTimeout/MaxConns) on the fasthttp.HostClient instance that uses uiDevServerAddr so dev-dashboard requests don’t block indefinitely — locate the uiDevClient variable initialization and add the timeout fields to that HostClient struct literal.Source: Coding guidelines
155-157: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winLog dev-proxy failures before falling back to embedded UI.
Proxy errors are currently swallowed, which makes local debugging harder when Vite is down or misconfigured.
🤖 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 `@transports/bifrost-http/handlers/ui.go` around lines 155 - 157, The call to uiDevClient.Do(&req, &resp) swallows errors and returns false without logging; update the error branch so it logs the error (including context like "dev-proxy request failed") before returning false. Specifically, inside the if err := uiDevClient.Do(&req, &resp); err != nil { ... } block, emit a log entry using the package/service logger available in this handler (e.g., logger, s.logger, or h.logger) that includes err and relevant request context (req.URL or method) and then return false.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/e2eseed/seed/seed.go`:
- Line 261: Remove the redundant InitEncryption(opts) call from SeedBase so
encryption initialization is performed by the caller or, alternatively, make
SeedBase responsible and remove the external call; specifically, either delete
the InitEncryption(opts) invocation in SeedBase and rely on the caller's
initialization, or if you prefer SeedBase to own init, document that SeedBase
calls encrypt.Init and remove the caller's InitEncryption usage—refer to the
InitEncryption function and the SeedBase entry point and adjust callers
accordingly to avoid double initialization of encrypt.Init.
- Around line 600-618: Replace usages of the local variable active and
address-taking (&active) in the TableVirtualKey literals with direct calls to
bifrost.Ptr(true); specifically update the IsActive fields in the virtual key
entries (the slice of tables.TableVirtualKey created where IDs like prefix +
"-vk-user-team", "-vk-team-only", "-vk-outside" are defined) to use
bifrost.Ptr(true) instead of &active, and remove the now-unused active := true
declaration.
---
Duplicate comments:
In `@transports/bifrost-http/handlers/ui.go`:
- Around line 16-19: uiDevClient currently constructs a fasthttp.HostClient with
no timeouts, which can hang requests; update the uiDevClient creation to set
sensible timeouts (e.g., ReadTimeout and WriteTimeout, and optionally
IdleTimeout/MaxConns) on the fasthttp.HostClient instance that uses
uiDevServerAddr so dev-dashboard requests don’t block indefinitely — locate the
uiDevClient variable initialization and add the timeout fields to that
HostClient struct literal.
- Around line 155-157: The call to uiDevClient.Do(&req, &resp) swallows errors
and returns false without logging; update the error branch so it logs the error
(including context like "dev-proxy request failed") before returning false.
Specifically, inside the if err := uiDevClient.Do(&req, &resp); err != nil { ...
} block, emit a log entry using the package/service logger available in this
handler (e.g., logger, s.logger, or h.logger) that includes err and relevant
request context (req.URL or method) and then return false.
In `@ui/app/_fallbacks/enterprise/components/agent/handoverView.tsx`:
- Around line 1-22: Rename the component file from handoverView.tsx to
PascalCase AgentHandoverView.tsx; keep the exported React component name
AgentHandoverView as-is, update all imports/usages that reference the old
filename (search for "handoverView" imports and update them to
"AgentHandoverView"), and ensure any route/fallback registrations or dynamic
imports that reference the filename are updated so the bundler/resolver picks up
the renamed file.
- Around line 5-6: The current useMemo with an empty dependency causes
status/isComplete to become stale; replace the useMemo-based read of
window.location.search with a reactive search params hook (e.g., useSearchParams
from next/navigation) or otherwise re-evaluate on param change: stop using
useMemo(..., []) and instead call const searchParams = useSearchParams(); const
status = searchParams.get("status"); const isComplete = !status || status ===
"complete"; (referencing the existing status and isComplete identifiers and
removing the useMemo usage).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1070855e-a1f9-4265-87e1-7f57ee28fe1f
⛔ Files ignored due to path filters (1)
cmd/e2eseed/go.sumis excluded by!**/*.sum
📒 Files selected for processing (9)
cmd/e2eseed/go.modcmd/e2eseed/main.gocmd/e2eseed/seed/seed.goplugins/governance/utils.gotests/cmd/e2eseed/main.gotransports/bifrost-http/handlers/ui.goui/app/_fallbacks/enterprise/components/agent/handoverView.tsxui/app/agent/handover/layout.tsxui/app/agent/handover/page.tsx
akshaydeo
force-pushed
the
06-04-fixed_seed_placements
branch
from
June 9, 2026 12:39
ed77ca6 to
09d6433
Compare
akshaydeo
force-pushed
the
06-04-fixed_seed_placements
branch
from
June 11, 2026 11:25
09d6433 to
6e10d42
Compare
akshaydeo
force-pushed
the
06-04-fixed_seed_placements
branch
from
June 12, 2026 13:31
6e10d42 to
0a6211b
Compare
akshaydeo
force-pushed
the
06-04-fixed_seed_placements
branch
2 times, most recently
from
June 13, 2026 13:22
60fc782 to
a620951
Compare
This was referenced Jun 13, 2026
akshaydeo
force-pushed
the
v2.0.0
branch
2 times, most recently
from
June 16, 2026 17:39
1480e3e to
73d20c1
Compare
## Summary Adds Helm chart support for two new Enterprise trace-publishing plugins: **Kafka** and **Google Cloud Pub/Sub**. These plugins allow Bifrost to publish completed traces as JSON messages to a Kafka topic or a GCP Pub/Sub topic respectively. ## Changes - Added Kafka plugin rendering logic to `_helpers.tpl`, supporting broker addresses, topic, SASL authentication, TLS, compression, batching, and span filtering options. - Added Pub/Sub plugin rendering logic to `_helpers.tpl`, supporting GCP project/topic IDs, service account key authentication (or ADC), auto topic creation, content logging controls, and span filtering. - Added JSON schema definitions for both `kafka` and `pubsub` plugin configs in `values.schema.json`, including field-level descriptions, enums, and `anyOf` patterns for env-var-substitutable fields. - Added conditional schema validation blocks in `transports/config.schema.json` for both plugins, enforcing required fields (`brokers`/`topic` for Kafka; `project_id`/`topic_id` for Pub/Sub). - Added default-disabled entries for both plugins in `values.yaml` with inline documentation comments covering all supported options. ## 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 Enable the Kafka plugin in your `values.yaml` and confirm the rendered config contains the expected plugin block: ```sh helm template bifrost ./helm-charts/bifrost \ --set bifrost.plugins.kafka.enabled=true \ --set bifrost.plugins.kafka.config.brokers[0]=localhost:9092 \ --set bifrost.plugins.kafka.config.topic=traces ``` Enable the Pub/Sub plugin and verify the rendered output: ```sh helm template bifrost ./helm-charts/bifrost \ --set bifrost.plugins.pubsub.enabled=true \ --set bifrost.plugins.pubsub.config.project_id=my-project \ --set bifrost.plugins.pubsub.config.topic_id=my-topic ``` Validate the schema against a config file: ```sh # Validate transports config schema npx ajv validate -s transports/config.schema.json -d <your-config.json> ``` **New config fields (Kafka):** | Field | Description | Default | |---|---|---| | `brokers` | Kafka broker addresses | required | | `topic` | Topic to publish traces to | required | | `sasl_enabled` | Enable SASL authentication | `false` | | `tls_enabled` | Enable TLS for broker connections | `false` | | `compression` | Codec: `none`, `gzip`, `snappy`, `lz4`, `zstd` | `none` | | `batch_size` | Max messages per batch | `100` | | `flush_interval_ms` | Max ms before flushing a batch | `1000` | | `auto_create_topic` | Create topic at startup if missing | `false` | | `disable_content_logging` | Strip message content from traces | `false` | **New config fields (Pub/Sub):** | Field | Description | Default | |---|---|---| | `project_id` | GCP project ID | required | | `topic_id` | Pub/Sub topic ID | required | | `service_account_key` | SA key JSON or `env.VAR`; omit for ADC | — | | `auto_create_topic` | Create topic at startup if missing | `false` | | `disable_content_logging` | Strip content from traces | `false` | ## Breaking changes - [ ] Yes - [x] No ## Security considerations - SASL credentials (`username`, `password`) and the Pub/Sub `service_account_key` support `env.VAR_NAME` substitution to avoid embedding secrets directly in config files. - TLS CA certificates for Kafka also support env-var substitution. - Both plugins support `disable_content_logging` to prevent PII or sensitive payload data from being published to the message broker. ## 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added Kafka and Pub/Sub as telemetry export backends for distributed tracing. * Both plugins are disabled by default and expose configuration for brokers/project/topic, optional auth (SASL/TLS/service account), compression/batching/flush, topic auto-creation, request header capture, and content/span filtering. * Chart/values and schema updates add configuration stubs and validation for these plugins. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
akshaydeo
force-pushed
the
06-04-fixed_seed_placements
branch
2 times, most recently
from
June 17, 2026 08:14
c7b3186 to
48b7cef
Compare
akshaydeo
force-pushed
the
06-04-fixed_seed_placements
branch
from
June 17, 2026 08:19
48b7cef to
39dfc83
Compare
Contributor
Author
Merge activity
|
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
3 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
Briefly explain the purpose of this PR and the problem it solves.
Changes
Type of change
Affected areas
How to test
Describe the steps to validate this change. Include commands and expected outcomes.
If adding new configs or environment variables, document them here.
Screenshots/Recordings
If UI changes, add before/after screenshots or short clips.
Breaking changes
If yes, describe impact and migration instructions.
Related issues
Link related issues and discussions. Example: Closes #123
Security considerations
Note any security implications (auth, secrets, PII, sandboxing, etc.).
Checklist
docs/contributing/README.mdand followed the guidelinesSummary by CodeRabbit
New Features
Chores