Conversation
|
Warning Rate limit exceeded@akshaydeo has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 2 minutes and 49 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (59)
📝 WalkthroughWalkthroughRefactors JSON serialization to use build-constrained native implementations (sonic for native, encoding/json for WASM/TinyGo), redesigns plugin HTTP transport API from middleware wrapping to direct interception with serializable Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Fasthttp Context
participant M as Transport Middleware
participant S as Server (apply helpers)
participant P as Plugin Loader / Plugin
rect rgba(135,206,235,0.12)
Note over M,P: Request serialization & interception flow
end
C->>M: incoming request
M->>M: getBifrostContextFromFastHTTP()
M->>M: fasthttpToHTTPRequest(ctx)
M->>P: loader.LoadDynamicPlugin/Plugin.HTTPTransportIntercept(ctxBifrost, req)
alt plugin returns (resp, nil)
P-->>M: *HTTPResponse
M->>S: applyHTTPResponseToCtx(resp)
S-->>C: short-circuit response (done)
else plugin returns (nil, nil)
P-->>M: (nil)
M->>M: applyHTTPRequestToCtx(req) (in-place changes)
M->>C: next handler
else plugin returns (nil, error)
P-->>M: error
M->>C: respond 500 + error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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 |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
6bd7ef1 to
afd603c
Compare
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
transports/bifrost-http/server/server.go (1)
218-243: Guard against nilPluginLoaderin dynamic plugin pathThe dynamic branch of
LoadPluginnow assumesbifrostConfig.PluginLoaderis non-nil:plugins, err := dynamicPlugins.LoadPlugins(bifrostConfig.PluginLoader, &dynamicPlugins.Config{...})If
LoadPluginis ever called withpath != niland a*lib.Configthat hasn’t hadPluginLoaderinitialized (e.g., tests or non-HTTP entrypoints), this will panic on a nil interface receiver.It’s safer to fail fast with a clear error:
Proposed guard
func LoadPlugin[T schemas.Plugin](ctx context.Context, name string, path *string, pluginConfig any, bifrostConfig *lib.Config) (T, error) { var zero T if path != nil { + if bifrostConfig == nil || bifrostConfig.PluginLoader == nil { + return zero, fmt.Errorf("dynamic plugin %s requested but PluginLoader is not configured", name) + } logger.Info("loading dynamic plugin %s from path %s", name, *path) - // Load dynamic plugin - plugins, err := dynamicPlugins.LoadPlugins(bifrostConfig.PluginLoader, &dynamicPlugins.Config{ + plugins, err := dynamicPlugins.LoadPlugins(bifrostConfig.PluginLoader, &dynamicPlugins.Config{ // ... })transports/bifrost-http/lib/config.go (1)
478-564: Delete unusedmergeClientConfigfunction and verify hash-based reconciliation during upgradesThe
loadClientConfigFromFilelogic is sound overall. Two notes:
mergeClientConfigis unused (no calls to it anywhere in the codebase)—delete it to avoid confusion about which path is authoritative.For upgrade behavior: migrations automatically backfill
ConfigHashfor existing DB rows beforeloadClientConfigFromFileruns, so the field won't be empty. However, verify that the hash-based comparison correctly handles cases where:
- The file config matches the current DB config (hash should match → DB config preserved, including UI changes)
- The file config differs from DB config (hash mismatch → file config overwrites DB)
Test edge cases where UI changes are made between versions to ensure the reconciliation logic respects the intended semantics.
🤖 Fix all issues with AI agents
In @.gitignore:
- Around line 55-57: There are duplicate "# Cursor specific" headers in
.gitignore; remove one and consolidate into a single "# Cursor specific" section
that contains both entries (.claude and .cursor/) so the file has one header and
both ignore lines beneath it, deleting the redundant header and duplicate entry.
In @core/schemas/context.go:
- Line 5: The file imports the fmt package but it's unused and will break the Go
build; open core/schemas/context.go and remove "fmt" from the import block (or
if you intended to use it, replace the unused import by actually calling fmt in
the relevant function such as any constructors or String methods), ensuring the
import list contains only used packages so the file compiles.
In @examples/plugins/hello-world-wasm-go/main.go:
- Around line 48-52: The code logs fields from input.Request without checking
for nil, which can cause a panic; update the logging in main.go to guard access
by first verifying input.Request != nil (and input.Request.Headers != nil before
indexing) and only then read Method, Path, and Headers, otherwise log a safe
fallback message indicating the request is missing; reference the variable input
and its Request and Headers fields when adding these nil checks.
In @examples/plugins/hello-world-wasm-go/memory.go:
- Around line 35-43: writeBytes currently allocates a Go-managed slice (buf)
which can be GC-collected, causing dangling pointers; change it to call
plugin_malloc(len(data)) to allocate in the plugin heap (managed via heapBase),
copy the contents of data into the plugin memory region, compute the pointer
(uint32) to the start of that plugin buffer, and then return packResult(ptr,
uint32(len(data))); ensure you do not retain a Go-only slice referencing the
plugin memory so there are no GC-managed references.
- Around line 18-21: The code in the heap growth branch relocates heapBase which
makes any raw pointers previously returned by plugin_malloc invalid (dangling);
instead either pre-allocate a fixed, sufficiently large heap in init (set
heapBase to a single large []byte and remove the resizing branch) or change the
allocation model so plugin_malloc returns an offset/index (uint32) into heapBase
rather than a Go pointer; ensure all code uses heapOffset and the stable
heapBase buffer (or offsets) so heapBase is never reallocated after pointers are
handed out.
In @examples/plugins/hello-world-wasm-go/README.md:
- Around line 5-23: Update the TinyGo version in README.md to a consistent,
current release: replace the pinned Linux package version string v0.32.0 with
v0.40.1 and make the macOS instruction explicit about installing that same
version (instead of an unconstrained `brew install tinygo`), so both platforms
reference the identical stable TinyGo release; update any download URLs or
package filenames (e.g., tinygo_0.32.0_amd64.deb) and the text around the TinyGo
installation to reflect v0.40.1.
In @examples/plugins/hello-world-wasm-rust/src/lib.rs:
- Around line 115-121: The PreHookOutput created in pre_hook currently sets
request to an empty JSON object; change it to preserve the incoming request by
assigning request: input.request.clone() (instead of serde_json::json!({})) when
building the PreHookOutput so the input.request is passed through; update the
PreHookOutput construction in the pre_hook function to use input.request.clone()
and keep context: input.context.clone() as-is.
- Around line 70-76: The output incorrectly sets request to an empty JSON
(serde_json::json!({})) instead of preserving the incoming request; update the
HTTPInterceptOutput construction to pass through the original request (e.g., use
request: input.request or request: input.request.clone() to satisfy ownership)
so the host receives the original request when unmodified, keeping context:
input.context and has_response: false unchanged.
In @examples/plugins/hello-world-wasm-typescript/assembly/types.ts:
- Around line 36-51: Add missing headers and query fields to align with the Go
SDK: update the HTTPRequest class to include a headers field and a query field,
and update HTTPResponse to include a headers field, using the same shape as the
Go types (headers and query should represent string->list-of-strings mappings);
if json-as cannot serialize Map/Dictionary types, replace them with a
serializable alternative (e.g., an array of key/value pairs or a custom
serializable record type) or document the limitation so Go interoperability
remains consistent with core/schemas/plugin.go.
In @framework/plugins/soloader.go:
- Around line 19-27: The URL check using strings.HasPrefix(dp.Path, "http") is
too permissive and downloaded temp files are leaked on failure; replace that
check with a proper scheme check (e.g., parse dp.Path with net/url.Parse and
accept only url.Scheme == "http" || "https" or explicitly use
strings.HasPrefix(dp.Path, "http://") || strings.HasPrefix(dp.Path,
"https://")), call DownloadPlugin(dp.Path, ".so") only when it really is an
HTTP(S) URL, and ensure any temp file created is removed if subsequent
operations (e.g., plugin.Open) fail — either by deferring os.Remove when
DownloadPlugin returns a temp path (and cancelling the defer on success) or by
removing the file in the error path after plugin.Open fails; reference dp.Path,
DownloadPlugin, and plugin.Open when applying the changes.
In @framework/plugins/soplugin.go:
- Around line 26-29: The GetName wrapper can panic if the underlying dp.getName
symbol is nil; update DynamicPlugin.GetName to check whether dp.getName is nil
and return a sensible fallback (e.g., the plugin filename or a default string)
when nil instead of calling it; locate the GetName method on type DynamicPlugin
and add a nil check against dp.getName, invoking dp.getName() only when non-nil
and otherwise returning the fallback.
In @transports/bifrost-http/handlers/middlewares.go:
- Around line 110-114: The loop over ctx.Request.URI().QueryArgs().All() is
invalid for fasthttp Args; replace it with
ctx.Request.URI().QueryArgs().VisitAll and inside the callback convert
keys/values to strings and store them into req.Query (use string(k)/string(v));
if you need to preserve multiple values per key, append new values to the
existing entry (e.g., join with a comma) instead of trying to iterate a
non-iterable map. Ensure you update the code that references req.Query
accordingly.
🧹 Nitpick comments (17)
examples/plugins/hello-world-wasm-go/memory.go (1)
28-29: Document the no-op free strategy.The no-op
plugin_freemeans allocated memory is never reclaimed during the plugin's lifetime. While this is acceptable for short-lived WASM plugins with bounded allocations, it would be helpful to document this design choice with a comment.📝 Suggested documentation
//export plugin_free -func plugin_free(ptr uint32) {} +func plugin_free(ptr uint32) { + // No-op: memory is reclaimed when the WASM instance is destroyed. + // For short-lived plugins with bounded memory usage, this is acceptable. +}examples/plugins/hello-world-wasm-rust/README.md (1)
48-53: Specify language for fenced code block.The fenced code block at line 48 should specify a language identifier for proper rendering and accessibility. For directory structures, use
textor leave blank.📝 Proposed fix
-``` +```text src/ ├── lib.rs # Plugin implementation (hooks) ├── memory.rs # Memory management utilities └── types.rs # Type definitions (mirrors Go SDK)</details> </blockquote></details> <details> <summary>examples/plugins/hello-world-wasm-typescript/README.md (1)</summary><blockquote> `42-48`: **Add language specifier to fenced code block.** The file structure code block lacks a language specifier. Consider adding `text` or `plaintext` for consistency with markdown best practices. <details> <summary>Suggested fix</summary> ```diff -``` +```text assembly/ ├── index.ts # Plugin implementation (hooks) ├── memory.ts # Memory management utilities ├── types.ts # Type definitions (mirrors Go SDK) └── tsconfig.json # AssemblyScript config</details> </blockquote></details> <details> <summary>examples/plugins/hello-world-wasm-rust/src/memory.rs (1)</summary><blockquote> `29-36`: **Consider whether lossy UTF-8 handling is appropriate.** `String::from_utf8_lossy` silently replaces invalid UTF-8 sequences with the replacement character (U+FFFD). For plugin data exchange, this could mask serialization issues or data corruption. Consider using `String::from_utf8` with explicit error handling to surface malformed input. <details> <summary>Alternative with error handling</summary> ```diff pub fn read_string(ptr: u32, len: u32) -> String { if len == 0 { return String::new(); } let bytes = unsafe { slice::from_raw_parts(ptr as *const u8, len as usize) }; - String::from_utf8_lossy(bytes).into_owned() + String::from_utf8(bytes.to_vec()).unwrap_or_else(|e| { + // Log or handle the error as appropriate + String::from_utf8_lossy(e.as_bytes()).into_owned() + }) }examples/plugins/hello-world-wasm-go/main.go (2)
62-63: Ignoringjson.Marshalerror.While unlikely to fail for these simple structs, ignoring the error from
json.Marshalis not ideal practice. Consider at minimum logging the error or using the error-returning pattern consistently.
84-87: Printinginterface{}values may produce unhelpful output.Using
printlnwithinterface{}values (vfrom the context map) may not produce meaningful output in WASM environments. Consider usingfmt.Sprintfor converting to string explicitly if debugging output is important.framework/plugins/loader.go (2)
5-8: Add documentation for the PluginLoader interface.While the interface design is clean and supports the dependency injection pattern for dynamic plugin loading, it lacks godoc comments explaining:
- The contract and expected behavior of
LoadDynamicPlugin- How the
pathparameter should be interpreted (local file, URL, etc.)- What configuration formats are expected in the
config anyparameter- Expected error conditions and handling
📝 Suggested documentation
+// PluginLoader is the contract for loading dynamic plugins at runtime. +// Implementations may support loading from local paths, URLs, or other sources. type PluginLoader interface { + // LoadDynamicPlugin loads a plugin from the specified path with the given configuration. + // The path interpretation depends on the loader implementation (e.g., file path, URL). + // The config parameter contains loader-specific configuration and may be nil. + // Returns the loaded plugin or an error if loading fails. LoadDynamicPlugin(path string, config any) (schemas.Plugin, error) }
7-7: Consider type safety tradeoff for the config parameter.The
config anyparameter provides flexibility for different loader implementations but sacrifices compile-time type safety. If specific loader implementations require different config types, callers may pass incorrect configurations that fail at runtime.This is likely an acceptable tradeoff for the abstraction, but consider documenting the expected config types for each loader implementation or using type assertions with clear error messages in implementations.
framework/plugins/soloader.go (2)
28-31: Variablepluginshadows the imported package name.This works but can cause confusion and accidental misuse if the code evolves.
Rename to avoid shadowing
- plugin, err := plugin.Open(dp.Path) + loadedPlugin, err := plugin.Open(dp.Path) if err != nil { return nil, err }Then update subsequent usages (
plugin.Lookup→loadedPlugin.Lookup).
60-67: Make HTTPTransportIntercept optional like Init to reduce boilerplate.Multiple plugins (telemetry, semanticcache, otel, mocker, maxim, logging, jsonparser) implement HTTPTransportIntercept as a no-op. Since the DynamicPlugin wrapper already handles nil gracefully, adopt the same pattern as Init: check for "symbol not found" error and allow plugins to omit this method.
framework/plugins/main.go (1)
16-19: Stray whitespace in struct definition.Line 17 appears to be an empty line inside the
Configstruct. While harmless, it's unusual formatting.Remove empty line
// Config is the configuration for the plugins framework type Config struct { - Plugins []DynamicPluginConfig `json:"plugins"` }framework/plugins/utils.go (1)
11-31: Consider adding integrity verification for downloaded plugins.Downloading and executing code from a URL is a significant security surface. Consider:
- Checksum/hash verification of the downloaded binary
- Signature verification for production deployments
- Restricting to HTTPS-only URLs
This may be acceptable for trusted internal deployments, but worth documenting the security implications.
Would you like me to propose an optional checksum verification mechanism, or should this be tracked as a future enhancement?
core/schemas/speech.go (1)
73-89: VoiceConfig object handling vs comment
UnmarshalJSONcurrently supports a plain string or a[]VoiceConfig, but not a singleVoiceConfigobject, while the comment still claims “VoiceConfig object/array”. If single-object input is a valid wire format anywhere, this will now hard-fail instead of being accepted.Two options:
- Either add a third branch to handle a lone
VoiceConfigobject (keeping behavior strictly additive), or- Narrow the comment to “string or array of VoiceConfig” so callers don’t assume object support.
Given the type already models multi-voice via a slice, adding the object branch is probably the more robust choice.
Also applies to: 94-121
transports/bifrost-http/server/server.go (1)
1197-1198: CentralizePluginLoaderdefault onlib.ConfigBootstrapping currently wires the loader via:
// Initializing plugin loader s.Config.PluginLoader = &dynamicPlugins.SharedObjectPluginLoader{}That means only configs created via this HTTP server path get a loader; other code constructing
lib.Config(tests, other transports, future CLIs) will see a nilPluginLoader, which breaks dynamic plugin loading.Consider moving this default into
lib.LoadConfig/loadConfigFromDefaults(or a small helper) so that anyConfigintended for dynamic plugins is initialized consistently, andLoadPlugin’s expectations are clearer.docs/plugins/writing-plugin.mdx (1)
89-98: Docs correctly updated forHTTPTransportInterceptAPIThe v1.4.x+ tab now matches the new transport hook signature and behavior: serializable
HTTPRequestpassed by pointer, in-place mutation, and clear short-circuit semantics for(nil, nil),(*HTTPResponse, nil), and(nil, error). The updated log example also reflects this.This looks consistent with the new plugin API and should be a good reference for authors.
Also applies to: 211-225, 432-436
framework/plugins/soplugin.go (1)
39-40: Misleading comments: "is not used for dynamic plugins".The comments on
PreHook,PostHook, andCleanupstate they are "not used for dynamic plugins", but the methods clearly delegate to the hook functions. These comments appear to be copy-paste errors or outdated.✏️ Suggested comment fix
-// PreHook is not used for dynamic plugins +// PreHook delegates to the plugin's preHook function func (dp *DynamicPlugin) PreHook(...) ... -// PostHook is not used for dynamic plugins +// PostHook delegates to the plugin's postHook function func (dp *DynamicPlugin) PostHook(...) ... -// Cleanup is not used for dynamic plugins +// Cleanup delegates to the plugin's cleanup function func (dp *DynamicPlugin) Cleanup() error {Also applies to: 44-45, 49-50
examples/plugins/hello-world-wasm-typescript/assembly/types.ts (1)
18-26: Context uses fixed fields instead of dynamic map.The Go SDK uses
map[string]interface{}for context, allowing arbitrary key-value pairs to flow through the plugin. This fixed-field approach limits the plugin to only these predefined keys, and any additional context keys from the host will be lost during JSON deserialization.Consider adding a comment documenting this limitation, or explore using
json-as'sJSON.Valuetype for dynamic handling if the library supports it.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
examples/plugins/hello-world-wasm-go/go.sumis excluded by!**/*.sumexamples/plugins/hello-world-wasm-typescript/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (60)
.gitignorecore/schemas/bifrost.gocore/schemas/chatcompletions.gocore/schemas/context.gocore/schemas/embedding.gocore/schemas/json_native.gocore/schemas/json_wasm.gocore/schemas/mcp.gocore/schemas/mcp_wasm.gocore/schemas/models.gocore/schemas/plugin.gocore/schemas/plugin_native.gocore/schemas/responses.gocore/schemas/speech.gocore/schemas/textcompletions.gocore/schemas/utils.godocs/enterprise/setting-up-okta.mdxdocs/plugins/migration-guide.mdxdocs/plugins/writing-plugin.mdxexamples/plugins/hello-world-wasm-go/Makefileexamples/plugins/hello-world-wasm-go/README.mdexamples/plugins/hello-world-wasm-go/go.modexamples/plugins/hello-world-wasm-go/main.goexamples/plugins/hello-world-wasm-go/memory.goexamples/plugins/hello-world-wasm-go/types.goexamples/plugins/hello-world-wasm-rust/Cargo.tomlexamples/plugins/hello-world-wasm-rust/Makefileexamples/plugins/hello-world-wasm-rust/README.mdexamples/plugins/hello-world-wasm-rust/src/lib.rsexamples/plugins/hello-world-wasm-rust/src/memory.rsexamples/plugins/hello-world-wasm-rust/src/types.rsexamples/plugins/hello-world-wasm-typescript/Makefileexamples/plugins/hello-world-wasm-typescript/README.mdexamples/plugins/hello-world-wasm-typescript/assembly/index.tsexamples/plugins/hello-world-wasm-typescript/assembly/memory.tsexamples/plugins/hello-world-wasm-typescript/assembly/tsconfig.jsonexamples/plugins/hello-world-wasm-typescript/assembly/types.tsexamples/plugins/hello-world-wasm-typescript/package.jsonexamples/plugins/hello-world/main.goframework/configstore/rdb.goframework/plugins/dynamicplugin.goframework/plugins/loader.goframework/plugins/main.goframework/plugins/soloader.goframework/plugins/soplugin.goframework/plugins/soplugin_test.goframework/plugins/utils.goplugins/governance/go.modplugins/governance/main.goplugins/jsonparser/main.goplugins/logging/main.goplugins/maxim/main.goplugins/mocker/main.goplugins/otel/main.goplugins/semanticcache/main.goplugins/telemetry/main.gotests/integrations/config.jsontransports/bifrost-http/handlers/middlewares.gotransports/bifrost-http/lib/config.gotransports/bifrost-http/server/server.go
💤 Files with no reviewable changes (1)
- framework/plugins/dynamicplugin.go
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
always check the stack if there is one for the current PR. do not give localized reviews for the PR, always see all changes in the light of the whole stack of PRs (if there is a stack, if there is no stack you can continue to make localized suggestions/reviews)
Files:
framework/plugins/loader.gotests/integrations/config.jsonexamples/plugins/hello-world-wasm-rust/Cargo.tomlcore/schemas/mcp_wasm.gocore/schemas/context.goexamples/plugins/hello-world-wasm-typescript/Makefileframework/plugins/utils.goexamples/plugins/hello-world-wasm-go/README.mdexamples/plugins/hello-world-wasm-rust/README.mdcore/schemas/utils.gocore/schemas/responses.godocs/plugins/migration-guide.mdxcore/schemas/chatcompletions.gocore/schemas/textcompletions.goframework/plugins/soloader.goplugins/mocker/main.goframework/plugins/main.goexamples/plugins/hello-world-wasm-rust/src/memory.rsplugins/semanticcache/main.goplugins/jsonparser/main.goplugins/maxim/main.gocore/schemas/json_wasm.goframework/configstore/rdb.gocore/schemas/speech.goexamples/plugins/hello-world-wasm-typescript/assembly/tsconfig.jsonplugins/telemetry/main.gotransports/bifrost-http/handlers/middlewares.goexamples/plugins/hello-world-wasm-go/types.gocore/schemas/models.goexamples/plugins/hello-world-wasm-rust/src/lib.rscore/schemas/mcp.goplugins/logging/main.goplugins/otel/main.goexamples/plugins/hello-world/main.gocore/schemas/plugin_native.gocore/schemas/bifrost.godocs/enterprise/setting-up-okta.mdxframework/plugins/soplugin.goplugins/governance/main.gocore/schemas/embedding.gotransports/bifrost-http/server/server.goexamples/plugins/hello-world-wasm-typescript/README.mdframework/plugins/soplugin_test.goexamples/plugins/hello-world-wasm-rust/Makefilecore/schemas/json_native.goexamples/plugins/hello-world-wasm-typescript/package.jsonexamples/plugins/hello-world-wasm-go/Makefiledocs/plugins/writing-plugin.mdxtransports/bifrost-http/lib/config.gocore/schemas/plugin.goplugins/governance/go.modexamples/plugins/hello-world-wasm-typescript/assembly/index.tsexamples/plugins/hello-world-wasm-go/go.modexamples/plugins/hello-world-wasm-go/memory.goexamples/plugins/hello-world-wasm-typescript/assembly/memory.tsexamples/plugins/hello-world-wasm-go/main.goexamples/plugins/hello-world-wasm-rust/src/types.rsexamples/plugins/hello-world-wasm-typescript/assembly/types.ts
🧠 Learnings (7)
📚 Learning: 2025-12-09T17:07:42.007Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/schemas/account.go:9-18
Timestamp: 2025-12-09T17:07:42.007Z
Learning: In core/schemas/account.go, the HuggingFaceKeyConfig field within the Key struct is currently unused and reserved for future Hugging Face inference endpoint deployments. Do not flag this field as missing from OpenAPI documentation or require its presence in the API spec until the feature is actively implemented and used. When the feature is added, update the OpenAPI docs accordingly; otherwise, treat this field as non-breaking and not part of the current API surface.
Applied to files:
framework/plugins/loader.gocore/schemas/mcp_wasm.gocore/schemas/context.goframework/plugins/utils.gocore/schemas/utils.gocore/schemas/responses.gocore/schemas/chatcompletions.gocore/schemas/textcompletions.goframework/plugins/soloader.goplugins/mocker/main.goframework/plugins/main.goplugins/semanticcache/main.goplugins/jsonparser/main.goplugins/maxim/main.gocore/schemas/json_wasm.goframework/configstore/rdb.gocore/schemas/speech.goplugins/telemetry/main.gotransports/bifrost-http/handlers/middlewares.goexamples/plugins/hello-world-wasm-go/types.gocore/schemas/models.gocore/schemas/mcp.goplugins/logging/main.goplugins/otel/main.goexamples/plugins/hello-world/main.gocore/schemas/plugin_native.gocore/schemas/bifrost.goframework/plugins/soplugin.goplugins/governance/main.gocore/schemas/embedding.gotransports/bifrost-http/server/server.goframework/plugins/soplugin_test.gocore/schemas/json_native.gotransports/bifrost-http/lib/config.gocore/schemas/plugin.goexamples/plugins/hello-world-wasm-go/memory.goexamples/plugins/hello-world-wasm-go/main.go
📚 Learning: 2025-12-29T11:54:55.836Z
Learnt from: akshaydeo
Repo: maximhq/bifrost PR: 1153
File: framework/configstore/rdb.go:2221-2246
Timestamp: 2025-12-29T11:54:55.836Z
Learning: In Go reviews, do not flag range-over-int patterns like for i := range n as compile-time errors, assuming Go 1.22+ semantics. Only flag actual range-capable values (slices, arrays, maps, channels, strings) and other compile-time issues. This applies to all Go files across the repository.
Applied to files:
framework/plugins/loader.gocore/schemas/mcp_wasm.gocore/schemas/context.goframework/plugins/utils.gocore/schemas/utils.gocore/schemas/responses.gocore/schemas/chatcompletions.gocore/schemas/textcompletions.goframework/plugins/soloader.goplugins/mocker/main.goframework/plugins/main.goplugins/semanticcache/main.goplugins/jsonparser/main.goplugins/maxim/main.gocore/schemas/json_wasm.goframework/configstore/rdb.gocore/schemas/speech.goplugins/telemetry/main.gotransports/bifrost-http/handlers/middlewares.goexamples/plugins/hello-world-wasm-go/types.gocore/schemas/models.gocore/schemas/mcp.goplugins/logging/main.goplugins/otel/main.goexamples/plugins/hello-world/main.gocore/schemas/plugin_native.gocore/schemas/bifrost.goframework/plugins/soplugin.goplugins/governance/main.gocore/schemas/embedding.gotransports/bifrost-http/server/server.goframework/plugins/soplugin_test.gocore/schemas/json_native.gotransports/bifrost-http/lib/config.gocore/schemas/plugin.goexamples/plugins/hello-world-wasm-go/memory.goexamples/plugins/hello-world-wasm-go/main.go
📚 Learning: 2025-12-30T05:37:48.365Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1180
File: docs/features/mcp/connecting-to-servers.mdx:452-458
Timestamp: 2025-12-30T05:37:48.365Z
Learning: When reviewing documentation PRs in a Graphite-managed stack, first check related or previous PRs in the stack for feature implementations before flagging documentation as incorrect or unsupported. Documentation MDX files often reference features implemented in earlier stack PRs; verify that the documented behavior exists in earlier changes and that the docs accurately reflect the implemented state before requesting edits.
Applied to files:
docs/plugins/migration-guide.mdxdocs/enterprise/setting-up-okta.mdxdocs/plugins/writing-plugin.mdx
📚 Learning: 2025-12-17T08:44:08.788Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1114
File: core/schemas/chatcompletions.go:224-228
Timestamp: 2025-12-17T08:44:08.788Z
Learning: In core/schemas/chatcompletions.go, ensure the schema structures mirror OpenAI's API specifications exactly. Use the valid values for fields like ChatAudioParameters.Format and ChatAudioParameters.Voice as defined by OpenAI's documentation, and avoid adding additional inline documentation or constants to maintain direct compatibility with OpenAI's API.
Applied to files:
core/schemas/chatcompletions.go
📚 Learning: 2025-12-12T08:25:02.629Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1000
File: transports/bifrost-http/integrations/router.go:709-712
Timestamp: 2025-12-12T08:25:02.629Z
Learning: In transports/bifrost-http/**/*.go, update streaming response handling to align with OpenAI Responses API: use typed SSE events such as response.created, response.output_text.delta, response.done, etc., and do not rely on the legacy data: [DONE] termination marker. Note that data: [DONE] is only used by the older Chat Completions and Text Completions streaming APIs. Ensure parsers, writers, and tests distinguish SSE events from the [DONE] sentinel and handle each event type accordingly for correct stream termination and progress updates.
Applied to files:
transports/bifrost-http/handlers/middlewares.gotransports/bifrost-http/server/server.gotransports/bifrost-http/lib/config.go
📚 Learning: 2025-12-29T09:14:16.633Z
Learnt from: akshaydeo
Repo: maximhq/bifrost PR: 888
File: transports/bifrost-http/handlers/middlewares.go:246-256
Timestamp: 2025-12-29T09:14:16.633Z
Learning: In the bifrost HTTP transport, fasthttp.RequestCtx is the primary context carrier and should be passed directly to functions that expect a context.Context. Do not convert to context.Context unless explicitly required. Ensure tracer implementations and related components are designed to accept fasthttp.RequestCtx directly, and document this architectural decision for maintainers.
Applied to files:
transports/bifrost-http/handlers/middlewares.gotransports/bifrost-http/server/server.gotransports/bifrost-http/lib/config.go
📚 Learning: 2025-12-22T10:50:40.990Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1154
File: plugins/governance/store.go:1165-1186
Timestamp: 2025-12-22T10:50:40.990Z
Learning: In the Bifrost governance plugin, budgets and rate limits have 1:1 relationships with their parent entities (virtual keys, teams, customers). Do not assume sharing; ensure cascade deletion logic only deletes budgets/rate limits when there are no shared references. Enforce invariants in code and add tests to verify no cross-entity sharing and that cascade deletes only remove the specific child of the parent. If a counterexample arises, adjust data model or add guards.
Applied to files:
plugins/governance/main.go
🧬 Code graph analysis (33)
framework/plugins/loader.go (1)
core/schemas/plugin.go (1)
Plugin(119-152)
core/schemas/utils.go (2)
core/schemas/json_native.go (1)
MarshalString(13-15)core/schemas/json_wasm.go (1)
MarshalString(13-19)
core/schemas/responses.go (2)
core/schemas/json_native.go (2)
Marshal(8-10)Unmarshal(18-20)core/schemas/json_wasm.go (2)
Marshal(8-10)Unmarshal(22-24)
core/schemas/chatcompletions.go (2)
core/schemas/json_native.go (2)
Unmarshal(18-20)Marshal(8-10)core/schemas/json_wasm.go (2)
Unmarshal(22-24)Marshal(8-10)
core/schemas/textcompletions.go (2)
core/schemas/json_native.go (2)
Marshal(8-10)Unmarshal(18-20)core/schemas/json_wasm.go (2)
Marshal(8-10)Unmarshal(22-24)
framework/plugins/soloader.go (5)
core/schemas/plugin.go (4)
Plugin(119-152)HTTPRequest(38-44)HTTPResponse(48-52)PluginShortCircuit(11-15)framework/plugins/soplugin.go (1)
DynamicPlugin(10-24)framework/plugins/utils.go (1)
DownloadPlugin(12-65)core/schemas/context.go (1)
BifrostContext(32-42)core/schemas/bifrost.go (3)
BifrostRequest(176-197)BifrostResponse(349-370)BifrostError(491-500)
plugins/mocker/main.go (3)
examples/plugins/hello-world/main.go (1)
HTTPTransportIntercept(18-26)core/schemas/context.go (1)
BifrostContext(32-42)core/schemas/plugin.go (2)
HTTPRequest(38-44)HTTPResponse(48-52)
framework/plugins/main.go (1)
framework/plugins/loader.go (1)
PluginLoader(6-8)
plugins/semanticcache/main.go (2)
core/schemas/plugin.go (3)
Plugin(119-152)HTTPRequest(38-44)HTTPResponse(48-52)examples/plugins/hello-world/main.go (1)
HTTPTransportIntercept(18-26)
plugins/jsonparser/main.go (3)
examples/plugins/hello-world/main.go (1)
HTTPTransportIntercept(18-26)core/schemas/context.go (1)
BifrostContext(32-42)core/schemas/plugin.go (2)
HTTPRequest(38-44)HTTPResponse(48-52)
plugins/maxim/main.go (3)
core/schemas/plugin.go (3)
Plugin(119-152)HTTPRequest(38-44)HTTPResponse(48-52)examples/plugins/hello-world/main.go (1)
HTTPTransportIntercept(18-26)core/schemas/context.go (1)
BifrostContext(32-42)
core/schemas/json_wasm.go (1)
core/schemas/json_native.go (3)
Marshal(8-10)MarshalString(13-15)Unmarshal(18-20)
core/schemas/speech.go (2)
core/schemas/json_native.go (2)
Marshal(8-10)Unmarshal(18-20)core/schemas/json_wasm.go (2)
Marshal(8-10)Unmarshal(22-24)
plugins/telemetry/main.go (3)
examples/plugins/hello-world/main.go (1)
HTTPTransportIntercept(18-26)core/schemas/context.go (1)
BifrostContext(32-42)core/schemas/plugin.go (2)
HTTPRequest(38-44)HTTPResponse(48-52)
transports/bifrost-http/handlers/middlewares.go (3)
core/schemas/plugin.go (4)
AcquireHTTPRequest(67-69)ReleaseHTTPRequest(74-86)HTTPRequest(38-44)HTTPResponse(48-52)core/schemas/context.go (2)
BifrostContext(32-42)NewBifrostContext(47-65)transports/bifrost-http/handlers/utils.go (1)
SendError(35-44)
core/schemas/models.go (2)
core/schemas/json_native.go (2)
Marshal(8-10)Unmarshal(18-20)core/schemas/json_wasm.go (2)
Marshal(8-10)Unmarshal(22-24)
examples/plugins/hello-world-wasm-rust/src/lib.rs (1)
examples/plugins/hello-world-wasm-rust/src/memory.rs (2)
read_string(30-36)write_string(14-27)
plugins/logging/main.go (3)
examples/plugins/hello-world/main.go (1)
HTTPTransportIntercept(18-26)core/schemas/context.go (1)
BifrostContext(32-42)core/schemas/plugin.go (2)
HTTPRequest(38-44)HTTPResponse(48-52)
plugins/otel/main.go (3)
examples/plugins/hello-world/main.go (1)
HTTPTransportIntercept(18-26)core/schemas/context.go (1)
BifrostContext(32-42)core/schemas/plugin.go (2)
HTTPRequest(38-44)HTTPResponse(48-52)
examples/plugins/hello-world/main.go (3)
core/schemas/context.go (1)
BifrostContext(32-42)core/schemas/plugin.go (2)
HTTPRequest(38-44)HTTPResponse(48-52)core/schemas/bifrost.go (1)
BifrostContextKey(118-118)
core/schemas/bifrost.go (2)
core/schemas/json_native.go (1)
Marshal(8-10)core/schemas/json_wasm.go (1)
Marshal(8-10)
framework/plugins/soplugin.go (3)
core/schemas/plugin.go (4)
Plugin(119-152)HTTPRequest(38-44)HTTPResponse(48-52)PluginShortCircuit(11-15)core/schemas/context.go (1)
BifrostContext(32-42)core/schemas/bifrost.go (3)
BifrostRequest(176-197)BifrostResponse(349-370)BifrostError(491-500)
plugins/governance/main.go (4)
core/schemas/context.go (1)
BifrostContext(32-42)core/schemas/plugin.go (2)
HTTPRequest(38-44)HTTPResponse(48-52)core/schemas/utils.go (1)
Ptr(14-16)core/schemas/json_native.go (2)
Unmarshal(18-20)Marshal(8-10)
core/schemas/embedding.go (2)
core/schemas/json_native.go (2)
Marshal(8-10)Unmarshal(18-20)core/schemas/json_wasm.go (2)
Marshal(8-10)Unmarshal(22-24)
transports/bifrost-http/server/server.go (4)
framework/plugins/main.go (2)
LoadPlugins(22-38)Config(16-19)framework/plugins/loader.go (1)
PluginLoader(6-8)transports/bifrost-http/lib/config.go (1)
Config(212-244)framework/plugins/soloader.go (1)
SharedObjectPluginLoader(12-12)
framework/plugins/soplugin_test.go (5)
framework/plugins/soloader.go (1)
SharedObjectPluginLoader(12-12)framework/plugins/main.go (1)
LoadPlugins(22-38)core/schemas/context.go (1)
NewBifrostContextWithTimeout(70-73)core/schemas/plugin.go (1)
HTTPRequest(38-44)examples/plugins/hello-world/main.go (1)
HTTPTransportIntercept(18-26)
core/schemas/json_native.go (1)
core/schemas/json_wasm.go (3)
Marshal(8-10)MarshalString(13-19)Unmarshal(22-24)
transports/bifrost-http/lib/config.go (3)
framework/plugins/loader.go (1)
PluginLoader(6-8)framework/configstore/clientconfig.go (1)
ClientConfig(37-55)framework/configstore/store.go (1)
ConfigStore(18-163)
core/schemas/plugin.go (2)
examples/plugins/hello-world/main.go (1)
HTTPTransportIntercept(18-26)core/schemas/context.go (1)
BifrostContext(32-42)
examples/plugins/hello-world-wasm-typescript/assembly/index.ts (3)
examples/plugins/hello-world-wasm-rust/src/memory.rs (2)
malloc(44-53)free(61-70)examples/plugins/hello-world-wasm-typescript/assembly/memory.ts (3)
malloc(33-39)free(42-45)writeString(13-20)examples/plugins/hello-world-wasm-go/types.go (6)
HTTPInterceptInput(10-13)HTTPInterceptOutput(16-22)PreHookInput(25-28)PreHookOutput(31-37)PostHookInput(40-45)PostHookOutput(48-54)
examples/plugins/hello-world-wasm-typescript/assembly/memory.ts (2)
examples/plugins/hello-world-wasm-rust/src/memory.rs (2)
malloc(44-53)free(61-70)examples/plugins/hello-world-wasm-typescript/assembly/index.ts (2)
malloc(29-31)free(33-35)
examples/plugins/hello-world-wasm-go/main.go (3)
examples/plugins/hello-world-wasm-go/types.go (6)
HTTPInterceptInput(10-13)HTTPInterceptOutput(16-22)PreHookInput(25-28)PreHookOutput(31-37)PostHookInput(40-45)PostHookOutput(48-54)core/schemas/json_native.go (2)
Unmarshal(18-20)Marshal(8-10)core/schemas/json_wasm.go (2)
Unmarshal(22-24)Marshal(8-10)
examples/plugins/hello-world-wasm-typescript/assembly/types.ts (6)
core/schemas/context.go (1)
BifrostContext(32-42)core/schemas/plugin.go (3)
HTTPRequest(38-44)HTTPResponse(48-52)PluginShortCircuit(11-15)examples/plugins/hello-world-wasm-go/types.go (6)
HTTPInterceptInput(10-13)HTTPInterceptOutput(16-22)PreHookInput(25-28)PreHookOutput(31-37)PostHookInput(40-45)PostHookOutput(48-54)core/schemas/chatcompletions.go (4)
ChatMessage(511-520)ChatParameters(154-184)BifrostChatRequest(10-17)BifrostChatResponse(25-40)core/schemas/bifrost.go (4)
BifrostRequest(176-197)BifrostResponse(349-370)ErrorField(509-516)BifrostError(491-500)ui/lib/types/logs.ts (1)
LLMUsage(205-211)
🪛 Biome (2.1.2)
examples/plugins/hello-world-wasm-typescript/assembly/index.ts
[error] 10-10: Do not shadow the global "JSON" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
examples/plugins/hello-world-wasm-typescript/assembly/types.ts
[error] 6-6: Do not shadow the global "JSON" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🪛 checkmake (0.2.2)
examples/plugins/hello-world-wasm-typescript/Makefile
[warning] 16-16: Target body for "help" exceeds allowed length of 5 (10).
(maxbodylength)
[warning] 58-58: Target body for "info" exceeds allowed length of 5 (10).
(maxbodylength)
[warning] 1-1: Missing required phony target "test"
(minphony)
examples/plugins/hello-world-wasm-rust/Makefile
[warning] 17-17: Target body for "help" exceeds allowed length of 5 (10).
(maxbodylength)
[warning] 39-39: Target body for "build" exceeds allowed length of 5 (6).
(maxbodylength)
[warning] 67-67: Target body for "info" exceeds allowed length of 5 (11).
(maxbodylength)
[warning] 1-1: Missing required phony target "test"
(minphony)
examples/plugins/hello-world-wasm-go/Makefile
[warning] 20-20: Target body for "help" exceeds allowed length of 5 (11).
(maxbodylength)
[warning] 60-60: Target body for "info" exceeds allowed length of 5 (12).
(maxbodylength)
[warning] 1-1: Missing required phony target "test"
(minphony)
🪛 markdownlint-cli2 (0.18.1)
examples/plugins/hello-world-wasm-rust/README.md
48-48: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
examples/plugins/hello-world-wasm-typescript/README.md
42-42: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
afd603c to
fbfd140
Compare
fbfd140 to
016cde0
Compare
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Fix all issues with AI agents
In @examples/plugins/hello-world-wasm-go/main.go:
- Around line 48-52: The code logs fields of input.Request without nil-checking,
which can panic if Request is nil; update the logging block (where println is
used to print "WASM Plugin: HTTP" and the content-type check) to first check if
input.Request != nil and only then access input.Request.Method and
input.Request.Headers, otherwise emit a safe message indicating the request is
missing; ensure the content-type lookup (input.Request.Headers["content-type"])
is guarded by the same nil-check so you never index into a nil Request or nil
Headers map.
In @examples/plugins/hello-world-wasm-go/Makefile:
- Around line 48-53: The Makefile’s build-optimized target uses the TinyGo flag
“-gc=leaking” which disables GC and can cause memory leaks; add a short comment
above the build-optimized target explaining that -gc=leaking disables garbage
collection, can leak memory, is only recommended for short-lived or
memory-constrained builds where binary size matters, and advise users to avoid
it for long-running plugins or to use the normal build target instead; reference
the build-optimized target and the -gc=leaking flag in the comment so
maintainers and users see the trade-off clearly.
In @examples/plugins/hello-world-wasm-go/memory.go:
- Around line 31-48: In plugin_malloc, the bounds check can overflow when
computing heapOffset+alignedSize; update the check to detect overflow before
adding (e.g., ensure heapOffset <= math.MaxUint32 - alignedSize or compute the
sum in a wider integer like uint64) and treat overflow as an allocation failure
(return 0) instead of proceeding; reference the plugin_malloc function and the
variables heapOffset, alignedSize, heapBase and heapBasePtr when locating and
fixing the check so you validate the sum safely before updating heapOffset or
computing ptr.
In @examples/plugins/hello-world-wasm-typescript/README.md:
- Around line 42-48: Add a language identifier to the fenced code block that
lists the assembly directory so it renders with proper syntax highlighting;
replace the opening triple backticks (```) before the directory tree with a
language tag such as ```text (or ```bash) so the block starts with ```text and
the existing tree lines (index.ts, memory.ts, types.ts, tsconfig.json) remain
unchanged.
In @framework/plugins/soplugin.go:
- Around line 50-52: The Cleanup method can panic if the dp.cleanup symbol is
nil; modify DynamicPlugin.Cleanup to nil-check dp.cleanup before calling it
(mirror the nil-safe pattern used in HTTPTransportIntercept), and if dp.cleanup
is nil return nil (no-op) instead of invoking it; update references to the
cleanup symbol (dp.cleanup and DynamicPlugin.Cleanup) to ensure the method
safely handles absent/unexported cleanup symbols.
- Around line 45-47: DynamicPlugin.PostHook currently calls dp.postHook directly
and can panic if dp.postHook is nil; update the method (DynamicPlugin.PostHook)
to first check if dp.postHook == nil and if so return the incoming resp,
bifrostErr, nil (pass-through), otherwise call and return dp.postHook(ctx, resp,
bifrostErr); mirror the nil-check pattern used in HTTPTransportIntercept to
avoid panics when the symbol is not exported or not set.
- Around line 26-29: The GetName method can panic if the plugin didn't export
the getName symbol; update DynamicPlugin.GetName to check whether dp.getName is
nil before invoking it and return a safe fallback (e.g., empty string or
"unknown") when nil; locate the GetName method and the getName symbol reference
and add the nil-check/early-return to avoid the panic and consider adding or
updating a unit test that ensures GetName returns the fallback when getName is
not present.
- Around line 40-42: PreHook currently calls dp.preHook directly and can panic
if the plugin didn't export that symbol; modify PreHook (method PreHook on type
DynamicPlugin) to first nil-check dp.preHook and, if nil, return the incoming
req with a nil PluginShortCircuit and nil error (pass-through behavior) similar
to HTTPTransportIntercept's handling, otherwise call and return dp.preHook(ctx,
req).
In @framework/plugins/utils.go:
- Around line 20-27: The response may be gzip-compressed because the request
sets Accept-Encoding: gzip, but fasthttp doesn't auto-decompress; after
fasthttp.DoTimeout returns in the code path using response (the Response
object), call response.BodyGunzip() or response.BodyUncompressed() and handle
its error before using response.Body() or writing the bytes to disk so the
plugin file is written with decompressed binary data; update the logic around
fasthttp.DoTimeout, response.BodyGunzip()/BodyUncompressed(), and subsequent
response.Body() usage to ensure decompression succeeds and errors are
propagated.
In @transports/bifrost-http/handlers/middlewares.go:
- Around line 104-107: The current loop uses ctx.Request.Header.All() which
doesn't exist on fasthttp.RequestHeader; replace it with
ctx.Request.Header.VisitAll to iterate headers. Use
ctx.Request.Header.VisitAll(func(k, v []byte) { req.Headers[string(k)] =
string(v) }) so keys and values are converted from []byte to string and stored
in req.Headers; update the code where the original loop was (the middleware
copying headers) to use VisitAll and remove the invalid All() usage.
- Around line 109-114: The loop over query params is using a non-existent All()
method; replace it with fasthttp's QueryArgs().VisitAll to iterate key/value
pairs and populate req.Query. Specifically, call
ctx.Request.URI().QueryArgs().VisitAll(func(k, v []byte) { req.Query[string(k)]
= string(v) }) (or append values if req.Query should hold multiple values),
removing the nested loop and using the VisitAll callback to convert byte slices
to strings.
🧹 Nitpick comments (14)
examples/plugins/hello-world-wasm-rust/Makefile (2)
39-59: Consider extracting the source WASM filename from Cargo.toml or using a variable.The hardcoded filename
hello_world_wasm_rust.wasmon lines 43 and 51 creates tight coupling between the Makefile and the crate name in Cargo.toml. If the crate name changes, the build will fail.♻️ Suggested approach
Option 1: Add a variable for the source filename
# Plugin configuration PLUGIN_NAME = hello-world +CARGO_WASM_NAME = hello_world_wasm_rust OUTPUT_DIR = build OUTPUT = $(OUTPUT_DIR)/$(PLUGIN_NAME).wasm TARGET = wasm32-unknown-unknownThen update the copy commands:
- @cp target/$(TARGET)/release/hello_world_wasm_rust.wasm $(OUTPUT) + @cp target/$(TARGET)/release/$(CARGO_WASM_NAME).wasm $(OUTPUT)Option 2: Extract from Cargo.toml dynamically
# Plugin configuration PLUGIN_NAME = hello-world +CARGO_WASM_NAME = $(shell grep '^name' Cargo.toml | head -1 | cut -d'"' -f2 | tr '-' '_') OUTPUT_DIR = buildThis extracts the crate name and converts hyphens to underscores to match Cargo's output naming convention.
1-1: Consider adding a test target for completeness.While the current build-focused targets are sufficient, adding a test target could improve the developer workflow for running Cargo tests.
Example implementation
-.PHONY: all build build-optimized clean help check-rust +.PHONY: all build build-optimized clean help check-rust testtest: check-rust ## Run tests @echo "$(COLOR_INFO)Running tests...$(COLOR_RESET)" cargo test @echo "$(COLOR_SUCCESS)✓ Tests complete$(COLOR_RESET)"examples/plugins/hello-world-wasm-typescript/Makefile (1)
58-70: LGTM: Informative status target and good default.The
infotarget provides helpful build status information with clear conditional logic, and setting the default goal tohelpis a best practice for user experience.Note: The static analysis warning about the
infotarget body exceeding 5 lines is a false positive—status reporting requires detailed output.Optional: Consider adding a test target.
For consistency with production-grade plugin examples, consider adding a
testtarget to run any validation or smoke tests for the WASM plugin. This would address the static analysis hint and provide a standard way to verify the plugin works as expected.💡 Example test target
+test: build ## Run plugin tests + @echo "$(COLOR_INFO)Running plugin tests...$(COLOR_RESET)" + @# Add test commands here (e.g., npm test or WASM runtime validation) + @echo "$(COLOR_SUCCESS)✓ Tests passed$(COLOR_RESET)" +Then update the phony declaration:
-.PHONY: all build build-debug clean help install check-node +.PHONY: all build build-debug clean help install check-node testexamples/plugins/hello-world-wasm-go/memory.go (1)
56-62: Consider adding defensive check or usage contract for plugin_reset.The function correctly resets the allocator, but the safety requirement ("should only be called when no allocated memory is in use") relies entirely on caller discipline. While this is acceptable for example code, consider documenting the expected usage pattern more explicitly or adding a debug assertion in development builds.
examples/plugins/hello-world-wasm-go/Makefile (1)
1-1: Consider completing the phony target declarations.The
.PHONYdeclaration is missing some targets that are defined later in the file (e.g.,info,build-optimized). While not critical, declaring all phony targets improves Makefile correctness.♻️ Suggested fix
-.PHONY: all build clean help check-tinygo +.PHONY: all build build-optimized clean help check-tinygo infoexamples/plugins/hello-world-wasm-rust/src/memory.rs (2)
44-53: Consider alignment requirements for allocated memory.The
mallocfunction uses a 1-byte alignment for all allocations. This could cause unaligned memory access issues if the WASM host writes multi-byte values (e.g., u32, u64, pointers) to this memory. Most platforms require proper alignment for performance or correctness.♻️ Proposed fix: Use a more standard alignment
- let layout = match Layout::from_size_align(size as usize, 1) { + let layout = match Layout::from_size_align(size as usize, 8) { Ok(l) => l, Err(_) => return 0, };And update
freeto match:- let layout = match Layout::from_size_align(size as usize, 1) { + let layout = match Layout::from_size_align(size as usize, 8) { Ok(l) => l, Err(_) => return, };Using 8-byte alignment ensures compatibility with most data types and matches common WASM memory alignment expectations.
38-42: Clarify safety documentation.The comment states "This function is marked as safe but performs unsafe operations internally," which is somewhat contradictory. While the function signature is indeed
pub extern "C" fn(notunsafe fn), the operations it performs are fundamentally unsafe because the caller must ensure proper usage (e.g., callingfreewith matching size, not using after free).📝 Suggested documentation improvement
-/// # Safety -/// This function is marked as safe but performs unsafe operations internally. -/// It is intended to be called from WASM host. +/// Allocate memory for the host to write data. +/// +/// # Safety Contract +/// Callers (typically the WASM host) must: +/// - Call `free` with the exact same size to deallocate +/// - Not use the pointer after calling `free` +/// - Ensure proper alignment of written dataSimilar clarification for
freefunction would be beneficial.examples/plugins/hello-world-wasm-go/main.go (1)
62-63: Handle JSON marshaling errors.The error from
json.Marshalis ignored on lines 62, 99, and 141. While marshaling of these structures is unlikely to fail, it's better to handle errors explicitly for robustness.♻️ Proposed improvement
- data, _ := json.Marshal(output) - return writeBytes(data) + data, err := json.Marshal(output) + if err != nil { + println("WASM Plugin: marshal error:", err.Error()) + return writeError("marshal error: " + err.Error()) + } + return writeBytes(data)Apply similar changes to lines 99 and 141.
core/schemas/responses.go (1)
787-803: Inconsistent null handling compared to rest of the file.Line 802 returns a raw null literal
[]byte("null"), while the rest of the file usesMarshal(nil)for null values (e.g., lines 162, 206, 366, 692, 989, 1288). For consistency and to ensure the null representation matches the chosen JSON library's behavior, consider updating this line.♻️ Proposed fix for consistency
// UnmarshalJSON implements custom JSON unmarshaling for ResponsesCodeInterpreterOutput func (o *ResponsesCodeInterpreterOutput) UnmarshalJSON(data []byte) error { // Handle null case if string(data) == "null" { return nil } // First, peek at the type field to determine which variant to unmarshal var typeStruct struct { Type string `json:"type"` } if err := Unmarshal(data, &typeStruct); err != nil { return fmt.Errorf("failed to read type field: %w", err) }Actually, the UnmarshalJSON is fine. The issue is in MarshalJSON:
// MarshalJSON implements custom JSON marshaling for ResponsesCodeInterpreterOutput func (o ResponsesCodeInterpreterOutput) MarshalJSON() ([]byte, error) { // Error if both variants are set if o.ResponsesCodeInterpreterOutputLogs != nil && o.ResponsesCodeInterpreterOutputImage != nil { return nil, fmt.Errorf("ResponsesCodeInterpreterOutput cannot have both Logs and Image set") } // Marshal whichever one is present if o.ResponsesCodeInterpreterOutputLogs != nil { return Marshal(o.ResponsesCodeInterpreterOutputLogs) } if o.ResponsesCodeInterpreterOutputImage != nil { return Marshal(o.ResponsesCodeInterpreterOutputImage) } // Return null if neither is set - return []byte("null"), nil + return Marshal(nil) }examples/plugins/hello-world-wasm-rust/README.md (1)
48-53: Add language specifier to fenced code block.The code block showing the file structure should have a language specifier for consistency and to satisfy markdown linting rules.
📝 Suggested fix
-``` +```text src/ ├── lib.rs # Plugin implementation (hooks) ├── memory.rs # Memory management utilities └── types.rs # Type definitions (mirrors Go SDK)</details> </blockquote></details> <details> <summary>framework/plugins/utils.go (2)</summary><blockquote> `12-31`: **Consider adding URL validation and download size limits.** The function accepts any URL without validation, which could allow: 1. Non-HTTP(S) schemes (e.g., `file://`, `ftp://`) 2. Unbounded downloads that could exhaust disk space <details> <summary>🛡️ Suggested validation</summary> ```diff func DownloadPlugin(url string, extension string) (string, error) { + // Validate URL scheme + if !strings.HasPrefix(url, "http://") && !strings.HasPrefix(url, "https://") { + return "", fmt.Errorf("invalid URL scheme: only http:// and https:// are supported") + } + req := fasthttp.AcquireRequest() defer fasthttp.ReleaseRequest(req) response := fasthttp.AcquireResponse() defer fasthttp.ReleaseResponse(response)You may also want to add a maximum download size check before writing to disk.
55-62: Consider setting executable permissions for WASM files as well.The permission check only handles
.sofiles, but WASM plugins (.wasm) might also benefit from having executable permissions set for consistency, though they don't strictly require it.transports/bifrost-http/handlers/middlewares.go (2)
124-131: Consider graceful handling instead of error response for path/method mismatch.The current implementation logs an error and sends an HTTP 409 response if a plugin modifies the method or path. However, this happens after the request has been processed by all plugins, which could leave the system in an inconsistent state. Consider either:
- Validating before processing (fail-fast)
- Documenting this as a contract violation that plugins must not make
94-97: Consider reusing existing BifrostContext from fasthttp context.
getBifrostContextFromFastHTTPalways creates a newBifrostContext. If one already exists in the fasthttp context (set by earlier middleware), it would be more efficient to retrieve and reuse it.♻️ Suggested enhancement
// getBifrostContextFromFastHTTP gets or creates a BifrostContext from fasthttp context. func getBifrostContextFromFastHTTP(ctx *fasthttp.RequestCtx) *schemas.BifrostContext { + // Try to get existing BifrostContext + if existing, ok := ctx.UserValue(schemas.BifrostContextKeyBifrostCtx).(*schemas.BifrostContext); ok && existing != nil { + return existing + } + // Create new and store for future use + bifrostCtx := schemas.NewBifrostContext(ctx, schemas.NoDeadline) + ctx.SetUserValue(schemas.BifrostContextKeyBifrostCtx, bifrostCtx) + return bifrostCtx - return schemas.NewBifrostContext(ctx, schemas.NoDeadline) }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
examples/plugins/hello-world-wasm-go/go.sumis excluded by!**/*.sumexamples/plugins/hello-world-wasm-typescript/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (59)
.gitignorecore/schemas/bifrost.gocore/schemas/chatcompletions.gocore/schemas/embedding.gocore/schemas/json_native.gocore/schemas/json_wasm.gocore/schemas/mcp.gocore/schemas/mcp_wasm.gocore/schemas/models.gocore/schemas/plugin.gocore/schemas/plugin_native.gocore/schemas/responses.gocore/schemas/speech.gocore/schemas/textcompletions.gocore/schemas/utils.godocs/enterprise/setting-up-okta.mdxdocs/plugins/migration-guide.mdxdocs/plugins/writing-plugin.mdxexamples/plugins/hello-world-wasm-go/Makefileexamples/plugins/hello-world-wasm-go/README.mdexamples/plugins/hello-world-wasm-go/go.modexamples/plugins/hello-world-wasm-go/main.goexamples/plugins/hello-world-wasm-go/memory.goexamples/plugins/hello-world-wasm-go/types.goexamples/plugins/hello-world-wasm-rust/Cargo.tomlexamples/plugins/hello-world-wasm-rust/Makefileexamples/plugins/hello-world-wasm-rust/README.mdexamples/plugins/hello-world-wasm-rust/src/lib.rsexamples/plugins/hello-world-wasm-rust/src/memory.rsexamples/plugins/hello-world-wasm-rust/src/types.rsexamples/plugins/hello-world-wasm-typescript/Makefileexamples/plugins/hello-world-wasm-typescript/README.mdexamples/plugins/hello-world-wasm-typescript/assembly/index.tsexamples/plugins/hello-world-wasm-typescript/assembly/memory.tsexamples/plugins/hello-world-wasm-typescript/assembly/tsconfig.jsonexamples/plugins/hello-world-wasm-typescript/assembly/types.tsexamples/plugins/hello-world-wasm-typescript/package.jsonexamples/plugins/hello-world/main.goframework/configstore/rdb.goframework/plugins/dynamicplugin.goframework/plugins/loader.goframework/plugins/main.goframework/plugins/soloader.goframework/plugins/soplugin.goframework/plugins/soplugin_test.goframework/plugins/utils.goplugins/governance/go.modplugins/governance/main.goplugins/jsonparser/main.goplugins/logging/main.goplugins/maxim/main.goplugins/mocker/main.goplugins/otel/main.goplugins/semanticcache/main.goplugins/telemetry/main.gotests/integrations/config.jsontransports/bifrost-http/handlers/middlewares.gotransports/bifrost-http/lib/config.gotransports/bifrost-http/server/server.go
💤 Files with no reviewable changes (1)
- framework/plugins/dynamicplugin.go
🚧 Files skipped from review as they are similar to previous changes (17)
- plugins/jsonparser/main.go
- examples/plugins/hello-world-wasm-typescript/assembly/tsconfig.json
- core/schemas/utils.go
- core/schemas/textcompletions.go
- core/schemas/bifrost.go
- core/schemas/plugin_native.go
- .gitignore
- examples/plugins/hello-world-wasm-typescript/package.json
- examples/plugins/hello-world-wasm-go/README.md
- core/schemas/mcp_wasm.go
- transports/bifrost-http/server/server.go
- examples/plugins/hello-world-wasm-rust/Cargo.toml
- plugins/governance/go.mod
- docs/enterprise/setting-up-okta.mdx
- plugins/maxim/main.go
- core/schemas/models.go
- examples/plugins/hello-world-wasm-go/types.go
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
always check the stack if there is one for the current PR. do not give localized reviews for the PR, always see all changes in the light of the whole stack of PRs (if there is a stack, if there is no stack you can continue to make localized suggestions/reviews)
Files:
framework/plugins/loader.goexamples/plugins/hello-world-wasm-typescript/README.mdcore/schemas/json_native.goexamples/plugins/hello-world-wasm-rust/Makefilecore/schemas/mcp.goplugins/semanticcache/main.goframework/plugins/main.goplugins/mocker/main.godocs/plugins/migration-guide.mdxframework/plugins/utils.godocs/plugins/writing-plugin.mdxplugins/telemetry/main.goplugins/logging/main.gocore/schemas/responses.gotests/integrations/config.jsonplugins/otel/main.goexamples/plugins/hello-world-wasm-go/Makefileframework/plugins/soplugin_test.gocore/schemas/embedding.goexamples/plugins/hello-world-wasm-go/memory.gotransports/bifrost-http/handlers/middlewares.goframework/plugins/soloader.goexamples/plugins/hello-world-wasm-typescript/assembly/memory.tsplugins/governance/main.gocore/schemas/chatcompletions.gocore/schemas/speech.goexamples/plugins/hello-world/main.goexamples/plugins/hello-world-wasm-rust/src/lib.rsexamples/plugins/hello-world-wasm-rust/src/memory.rsframework/plugins/soplugin.goexamples/plugins/hello-world-wasm-go/main.goframework/configstore/rdb.gotransports/bifrost-http/lib/config.gocore/schemas/plugin.gocore/schemas/json_wasm.goexamples/plugins/hello-world-wasm-rust/src/types.rsexamples/plugins/hello-world-wasm-go/go.modexamples/plugins/hello-world-wasm-typescript/assembly/index.tsexamples/plugins/hello-world-wasm-rust/README.mdexamples/plugins/hello-world-wasm-typescript/Makefileexamples/plugins/hello-world-wasm-typescript/assembly/types.ts
🧠 Learnings (7)
📚 Learning: 2025-12-09T17:07:42.007Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/schemas/account.go:9-18
Timestamp: 2025-12-09T17:07:42.007Z
Learning: In core/schemas/account.go, the HuggingFaceKeyConfig field within the Key struct is currently unused and reserved for future Hugging Face inference endpoint deployments. Do not flag this field as missing from OpenAPI documentation or require its presence in the API spec until the feature is actively implemented and used. When the feature is added, update the OpenAPI docs accordingly; otherwise, treat this field as non-breaking and not part of the current API surface.
Applied to files:
framework/plugins/loader.gocore/schemas/json_native.gocore/schemas/mcp.goplugins/semanticcache/main.goframework/plugins/main.goplugins/mocker/main.goframework/plugins/utils.goplugins/telemetry/main.goplugins/logging/main.gocore/schemas/responses.goplugins/otel/main.goframework/plugins/soplugin_test.gocore/schemas/embedding.goexamples/plugins/hello-world-wasm-go/memory.gotransports/bifrost-http/handlers/middlewares.goframework/plugins/soloader.goplugins/governance/main.gocore/schemas/chatcompletions.gocore/schemas/speech.goexamples/plugins/hello-world/main.goframework/plugins/soplugin.goexamples/plugins/hello-world-wasm-go/main.goframework/configstore/rdb.gotransports/bifrost-http/lib/config.gocore/schemas/plugin.gocore/schemas/json_wasm.go
📚 Learning: 2025-12-29T11:54:55.836Z
Learnt from: akshaydeo
Repo: maximhq/bifrost PR: 1153
File: framework/configstore/rdb.go:2221-2246
Timestamp: 2025-12-29T11:54:55.836Z
Learning: In Go reviews, do not flag range-over-int patterns like for i := range n as compile-time errors, assuming Go 1.22+ semantics. Only flag actual range-capable values (slices, arrays, maps, channels, strings) and other compile-time issues. This applies to all Go files across the repository.
Applied to files:
framework/plugins/loader.gocore/schemas/json_native.gocore/schemas/mcp.goplugins/semanticcache/main.goframework/plugins/main.goplugins/mocker/main.goframework/plugins/utils.goplugins/telemetry/main.goplugins/logging/main.gocore/schemas/responses.goplugins/otel/main.goframework/plugins/soplugin_test.gocore/schemas/embedding.goexamples/plugins/hello-world-wasm-go/memory.gotransports/bifrost-http/handlers/middlewares.goframework/plugins/soloader.goplugins/governance/main.gocore/schemas/chatcompletions.gocore/schemas/speech.goexamples/plugins/hello-world/main.goframework/plugins/soplugin.goexamples/plugins/hello-world-wasm-go/main.goframework/configstore/rdb.gotransports/bifrost-http/lib/config.gocore/schemas/plugin.gocore/schemas/json_wasm.go
📚 Learning: 2025-12-30T05:37:48.365Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1180
File: docs/features/mcp/connecting-to-servers.mdx:452-458
Timestamp: 2025-12-30T05:37:48.365Z
Learning: When reviewing documentation PRs in a Graphite-managed stack, first check related or previous PRs in the stack for feature implementations before flagging documentation as incorrect or unsupported. Documentation MDX files often reference features implemented in earlier stack PRs; verify that the documented behavior exists in earlier changes and that the docs accurately reflect the implemented state before requesting edits.
Applied to files:
docs/plugins/migration-guide.mdxdocs/plugins/writing-plugin.mdx
📚 Learning: 2025-12-12T08:25:02.629Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1000
File: transports/bifrost-http/integrations/router.go:709-712
Timestamp: 2025-12-12T08:25:02.629Z
Learning: In transports/bifrost-http/**/*.go, update streaming response handling to align with OpenAI Responses API: use typed SSE events such as response.created, response.output_text.delta, response.done, etc., and do not rely on the legacy data: [DONE] termination marker. Note that data: [DONE] is only used by the older Chat Completions and Text Completions streaming APIs. Ensure parsers, writers, and tests distinguish SSE events from the [DONE] sentinel and handle each event type accordingly for correct stream termination and progress updates.
Applied to files:
transports/bifrost-http/handlers/middlewares.gotransports/bifrost-http/lib/config.go
📚 Learning: 2025-12-29T09:14:16.633Z
Learnt from: akshaydeo
Repo: maximhq/bifrost PR: 888
File: transports/bifrost-http/handlers/middlewares.go:246-256
Timestamp: 2025-12-29T09:14:16.633Z
Learning: In the bifrost HTTP transport, fasthttp.RequestCtx is the primary context carrier and should be passed directly to functions that expect a context.Context. Do not convert to context.Context unless explicitly required. Ensure tracer implementations and related components are designed to accept fasthttp.RequestCtx directly, and document this architectural decision for maintainers.
Applied to files:
transports/bifrost-http/handlers/middlewares.gotransports/bifrost-http/lib/config.go
📚 Learning: 2025-12-22T10:50:40.990Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1154
File: plugins/governance/store.go:1165-1186
Timestamp: 2025-12-22T10:50:40.990Z
Learning: In the Bifrost governance plugin, budgets and rate limits have 1:1 relationships with their parent entities (virtual keys, teams, customers). Do not assume sharing; ensure cascade deletion logic only deletes budgets/rate limits when there are no shared references. Enforce invariants in code and add tests to verify no cross-entity sharing and that cascade deletes only remove the specific child of the parent. If a counterexample arises, adjust data model or add guards.
Applied to files:
plugins/governance/main.go
📚 Learning: 2025-12-17T08:44:08.788Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1114
File: core/schemas/chatcompletions.go:224-228
Timestamp: 2025-12-17T08:44:08.788Z
Learning: In core/schemas/chatcompletions.go, ensure the schema structures mirror OpenAI's API specifications exactly. Use the valid values for fields like ChatAudioParameters.Format and ChatAudioParameters.Voice as defined by OpenAI's documentation, and avoid adding additional inline documentation or constants to maintain direct compatibility with OpenAI's API.
Applied to files:
core/schemas/chatcompletions.go
🧬 Code graph analysis (20)
framework/plugins/loader.go (1)
core/schemas/plugin.go (1)
Plugin(119-152)
core/schemas/json_native.go (1)
core/schemas/json_wasm.go (3)
Marshal(8-10)MarshalString(13-19)Unmarshal(22-24)
framework/plugins/main.go (1)
framework/plugins/loader.go (1)
PluginLoader(6-8)
plugins/mocker/main.go (3)
examples/plugins/hello-world/main.go (1)
HTTPTransportIntercept(18-26)core/schemas/context.go (1)
BifrostContext(31-41)core/schemas/plugin.go (2)
HTTPRequest(38-44)HTTPResponse(48-52)
plugins/telemetry/main.go (3)
examples/plugins/hello-world/main.go (1)
HTTPTransportIntercept(18-26)core/schemas/context.go (1)
BifrostContext(31-41)core/schemas/plugin.go (2)
HTTPRequest(38-44)HTTPResponse(48-52)
plugins/logging/main.go (3)
examples/plugins/hello-world/main.go (1)
HTTPTransportIntercept(18-26)core/schemas/context.go (1)
BifrostContext(31-41)core/schemas/plugin.go (2)
HTTPRequest(38-44)HTTPResponse(48-52)
plugins/otel/main.go (3)
examples/plugins/hello-world/main.go (1)
HTTPTransportIntercept(18-26)core/schemas/context.go (1)
BifrostContext(31-41)core/schemas/plugin.go (2)
HTTPRequest(38-44)HTTPResponse(48-52)
core/schemas/embedding.go (2)
core/schemas/json_native.go (2)
Marshal(8-10)Unmarshal(18-20)core/schemas/json_wasm.go (2)
Marshal(8-10)Unmarshal(22-24)
framework/plugins/soloader.go (5)
core/schemas/plugin.go (4)
Plugin(119-152)HTTPRequest(38-44)HTTPResponse(48-52)PluginShortCircuit(11-15)framework/plugins/soplugin.go (1)
DynamicPlugin(10-24)framework/plugins/utils.go (1)
DownloadPlugin(12-65)core/schemas/context.go (1)
BifrostContext(31-41)core/schemas/bifrost.go (3)
BifrostRequest(176-197)BifrostResponse(349-370)BifrostError(491-500)
examples/plugins/hello-world-wasm-typescript/assembly/memory.ts (2)
examples/plugins/hello-world-wasm-rust/src/memory.rs (2)
malloc(44-53)free(61-70)examples/plugins/hello-world-wasm-typescript/assembly/index.ts (2)
malloc(29-31)free(33-35)
core/schemas/chatcompletions.go (2)
core/schemas/json_native.go (2)
Unmarshal(18-20)Marshal(8-10)core/schemas/json_wasm.go (2)
Unmarshal(22-24)Marshal(8-10)
core/schemas/speech.go (2)
core/schemas/json_native.go (2)
Marshal(8-10)Unmarshal(18-20)core/schemas/json_wasm.go (2)
Marshal(8-10)Unmarshal(22-24)
examples/plugins/hello-world/main.go (3)
core/schemas/context.go (1)
BifrostContext(31-41)core/schemas/plugin.go (2)
HTTPRequest(38-44)HTTPResponse(48-52)core/schemas/bifrost.go (1)
BifrostContextKey(118-118)
examples/plugins/hello-world-wasm-rust/src/lib.rs (3)
examples/plugins/hello-world-wasm-rust/src/memory.rs (2)
read_string(30-36)write_string(14-27)core/schemas/plugin.go (1)
PluginConfig(156-162)examples/plugins/hello-world-wasm-rust/src/types.rs (4)
default(129-131)default(143-145)new(26-28)new(423-431)
framework/plugins/soplugin.go (4)
core/schemas/plugin.go (4)
Plugin(119-152)HTTPRequest(38-44)HTTPResponse(48-52)PluginShortCircuit(11-15)core/schemas/context.go (1)
BifrostContext(31-41)core/schemas/bifrost.go (3)
BifrostRequest(176-197)BifrostResponse(349-370)BifrostError(491-500)examples/plugins/hello-world/main.go (5)
GetName(14-16)HTTPTransportIntercept(18-26)PreHook(28-34)PostHook(36-43)Cleanup(45-48)
examples/plugins/hello-world-wasm-go/main.go (3)
examples/plugins/hello-world-wasm-go/types.go (6)
HTTPInterceptInput(10-13)HTTPInterceptOutput(16-22)PreHookInput(25-28)PreHookOutput(31-37)PostHookInput(40-45)PostHookOutput(48-54)core/schemas/json_native.go (2)
Unmarshal(18-20)Marshal(8-10)core/schemas/json_wasm.go (2)
Unmarshal(22-24)Marshal(8-10)
transports/bifrost-http/lib/config.go (3)
framework/plugins/loader.go (1)
PluginLoader(6-8)framework/configstore/clientconfig.go (1)
ClientConfig(37-55)framework/configstore/store.go (1)
ConfigStore(18-163)
core/schemas/plugin.go (2)
examples/plugins/hello-world/main.go (1)
HTTPTransportIntercept(18-26)core/schemas/context.go (1)
BifrostContext(31-41)
core/schemas/json_wasm.go (1)
core/schemas/json_native.go (3)
Marshal(8-10)MarshalString(13-15)Unmarshal(18-20)
examples/plugins/hello-world-wasm-typescript/assembly/index.ts (3)
examples/plugins/hello-world-wasm-rust/src/memory.rs (2)
malloc(44-53)free(61-70)examples/plugins/hello-world-wasm-typescript/assembly/memory.ts (4)
malloc(33-39)free(42-45)writeString(13-20)readString(23-30)examples/plugins/hello-world-wasm-go/types.go (6)
HTTPInterceptInput(10-13)HTTPInterceptOutput(16-22)PreHookInput(25-28)PreHookOutput(31-37)PostHookInput(40-45)PostHookOutput(48-54)
🪛 Biome (2.1.2)
examples/plugins/hello-world-wasm-typescript/assembly/index.ts
[error] 10-10: Do not shadow the global "JSON" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
examples/plugins/hello-world-wasm-typescript/assembly/types.ts
[error] 6-6: Do not shadow the global "JSON" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🪛 checkmake (0.2.2)
examples/plugins/hello-world-wasm-rust/Makefile
[warning] 1-1: Missing required phony target "test"
(minphony)
[warning] 17-17: Target body for "help" exceeds allowed length of 5 (10).
(maxbodylength)
[warning] 39-39: Target body for "build" exceeds allowed length of 5 (6).
(maxbodylength)
[warning] 67-67: Target body for "info" exceeds allowed length of 5 (11).
(maxbodylength)
examples/plugins/hello-world-wasm-go/Makefile
[warning] 1-1: Missing required phony target "test"
(minphony)
[warning] 20-20: Target body for "help" exceeds allowed length of 5 (11).
(maxbodylength)
[warning] 60-60: Target body for "info" exceeds allowed length of 5 (12).
(maxbodylength)
examples/plugins/hello-world-wasm-typescript/Makefile
[warning] 16-16: Target body for "help" exceeds allowed length of 5 (10).
(maxbodylength)
[warning] 58-58: Target body for "info" exceeds allowed length of 5 (10).
(maxbodylength)
[warning] 1-1: Missing required phony target "test"
(minphony)
🪛 LanguageTool
examples/plugins/hello-world-wasm-rust/README.md
[style] ~524-~524: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...ies**: Rust WASM binaries are typically very small 4. Cross-platform: Single .wasm b...
(EN_WEAK_ADJECTIVE)
🪛 markdownlint-cli2 (0.18.1)
examples/plugins/hello-world-wasm-typescript/README.md
42-42: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
examples/plugins/hello-world-wasm-rust/README.md
48-48: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
016cde0 to
511a030
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
framework/plugins/main.go (1)
22-37: Add nil check for loader parameter.If
LoadPluginsis called with anilloader and there are enabled plugins, it will panic with a nil pointer dereference at line 31. Consider adding a defensive check:Suggested fix
func LoadPlugins(loader PluginLoader, config *Config) ([]schemas.Plugin, error) { plugins := []schemas.Plugin{} if config == nil { return plugins, nil } + if loader == nil && len(config.Plugins) > 0 { + // Check if any plugins are enabled + for _, dp := range config.Plugins { + if dp.Enabled { + return nil, fmt.Errorf("plugin loader is nil but enabled plugins exist") + } + } + } for _, dp := range config.Plugins { if !dp.Enabled { continue } plugin, err := loader.LoadDynamicPlugin(dp.Path, dp.Config)Or more simply, check at the point of use:
for _, dp := range config.Plugins { if !dp.Enabled { continue } + if loader == nil { + return nil, fmt.Errorf("plugin loader is required to load enabled plugins") + } plugin, err := loader.LoadDynamicPlugin(dp.Path, dp.Config)
🤖 Fix all issues with AI agents
In @docs/enterprise/setting-up-okta.mdx:
- Around line 231-233: The note about role claims is misleading—update the Note
after steps 4-5 to clarify that role claims require configuring custom claims on
an authorization server (e.g. the /oauth2/default server when it has custom
claims enabled) and that Okta’s built-in org authorization server does not
support custom claims; alternatively reword to state “role claims are available
only when you configure custom claims on your authorization server (the built‑in
org server does not support this), so ensure you add role claims to your chosen
authorization server (for example /oauth2/default) to allow RBAC and Admin
assignment via the Bifrost dashboard.”
- Around line 74-80: Update the Step 3 text and the Note to clarify that the
default authorization server (/oauth2/default) is available to all Okta plans
and does support custom claims (including role claims) without the API Access
Management paid add-on; state the paid add-on is only needed to create
additional custom authorization servers, and explicitly describe the three
workflows: use /oauth2/default and perform Steps 4-7 to configure role claims,
skip Steps 4-7 and accept the default-first-user Admin assignment, or skip Step
3 entirely which means authorization is not configured.
In @examples/plugins/hello-world-wasm-rust/README.md:
- Around line 48-53: The README's fenced code block showing the src/ directory
tree lacks a language identifier, so update the triple-backtick fence that
surrounds the file tree to include a language (e.g., "text" or "bash") so the
directory listing is rendered and highlighted correctly; locate the fenced block
in the README.md that contains the lines starting with "src/" and change the
opening ``` to ```text (or another appropriate identifier).
In @examples/plugins/hello-world-wasm-typescript/assembly/types.ts:
- Around line 36-50: The HTTPRequest and HTTPResponse classes are missing
headers and query fields needed to mirror the Go SDK; update the HTTPRequest
class (symbol: HTTPRequest) to add headers: Map<string,string> = new Map() and
query: Map<string,string> = new Map(), and update the HTTPResponse class
(symbol: HTTPResponse) to add headers: Map<string,string> = new Map(); keep
existing defaults and types, and use json-as compatible Map serialization so
plugins can inspect/modify HTTP headers and query params.
In @transports/bifrost-http/handlers/middlewares.go:
- Around line 110-114: The loop incorrectly treats the second element from
ctx.Request.URI().QueryArgs().All() as a slice of values and iterates its bytes;
instead, for each pair (key, valBytes) yielded by All() assign
req.Query[string(key)] = string(valBytes) directly. Replace the nested loop over
values with a single assignment using the byte slice returned by All(), keeping
the variable names (key, values/valBytes) and updating req.Query accordingly.
🧹 Nitpick comments (4)
framework/plugins/utils.go (2)
20-21: RedundantAccept-Languageheader for binary download.The
Accept-Languageheader is typically used for content negotiation of human-readable content. For binary plugin downloads, this header is unnecessary.Optional cleanup
req.Header.Set("Accept", "application/octet-stream") req.Header.Set("Accept-Encoding", "gzip") - req.Header.Set("Accept-Language", "en-US,en;q=0.9")
11-72: Add integrity verification for downloaded plugins.The function downloads and loads arbitrary binaries from remote URLs without validation. This is a security risk since:
- Downloaded plugins receive executable permissions (0755 for .so files) and are dynamically loaded
- No checksum, signature, or URL validation exists anywhere in the plugin loading flow
- Only structural validation occurs after download (checking for required methods)
Consider implementing:
- Checksum or cryptographic signature verification of downloaded plugins
- URL allowlist for trusted plugin sources
- Audit logging of plugin downloads (source URL and integrity hash)
framework/plugins/main.go (1)
16-19: Minor: Empty line in struct definition.There's an empty line at line 17 within the
Configstruct definition. This is a minor formatting inconsistency.Optional cleanup
// Config is the configuration for the plugins framework type Config struct { - Plugins []DynamicPluginConfig `json:"plugins"` }docs/enterprise/setting-up-okta.mdx (1)
250-250: Consider restoring the example Issuer URL for clarity.Changing from a specific example (likely
https://your-domain.okta.com/oauth2/default) to the generic label "Issuer URL" reduces clarity in the step-by-step instructions. While Line 265 provides the example in the Configuration Reference table, users benefit from seeing concrete examples at each step.Suggestion: Consider reverting to show the example format:
Your Okta authorization server URL (e.g., https://your-domain.okta.com/oauth2/default).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
examples/plugins/hello-world-wasm-go/go.sumis excluded by!**/*.sumexamples/plugins/hello-world-wasm-typescript/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (59)
.gitignorecore/schemas/bifrost.gocore/schemas/chatcompletions.gocore/schemas/embedding.gocore/schemas/json_native.gocore/schemas/json_wasm.gocore/schemas/mcp.gocore/schemas/mcp_wasm.gocore/schemas/models.gocore/schemas/plugin.gocore/schemas/plugin_native.gocore/schemas/responses.gocore/schemas/speech.gocore/schemas/textcompletions.gocore/schemas/utils.godocs/enterprise/setting-up-okta.mdxdocs/plugins/migration-guide.mdxdocs/plugins/writing-plugin.mdxexamples/plugins/hello-world-wasm-go/Makefileexamples/plugins/hello-world-wasm-go/README.mdexamples/plugins/hello-world-wasm-go/go.modexamples/plugins/hello-world-wasm-go/main.goexamples/plugins/hello-world-wasm-go/memory.goexamples/plugins/hello-world-wasm-go/types.goexamples/plugins/hello-world-wasm-rust/Cargo.tomlexamples/plugins/hello-world-wasm-rust/Makefileexamples/plugins/hello-world-wasm-rust/README.mdexamples/plugins/hello-world-wasm-rust/src/lib.rsexamples/plugins/hello-world-wasm-rust/src/memory.rsexamples/plugins/hello-world-wasm-rust/src/types.rsexamples/plugins/hello-world-wasm-typescript/Makefileexamples/plugins/hello-world-wasm-typescript/README.mdexamples/plugins/hello-world-wasm-typescript/assembly/index.tsexamples/plugins/hello-world-wasm-typescript/assembly/memory.tsexamples/plugins/hello-world-wasm-typescript/assembly/tsconfig.jsonexamples/plugins/hello-world-wasm-typescript/assembly/types.tsexamples/plugins/hello-world-wasm-typescript/package.jsonexamples/plugins/hello-world/main.goframework/configstore/rdb.goframework/plugins/dynamicplugin.goframework/plugins/loader.goframework/plugins/main.goframework/plugins/soloader.goframework/plugins/soplugin.goframework/plugins/soplugin_test.goframework/plugins/utils.goplugins/governance/go.modplugins/governance/main.goplugins/jsonparser/main.goplugins/logging/main.goplugins/maxim/main.goplugins/mocker/main.goplugins/otel/main.goplugins/semanticcache/main.goplugins/telemetry/main.gotests/integrations/config.jsontransports/bifrost-http/handlers/middlewares.gotransports/bifrost-http/lib/config.gotransports/bifrost-http/server/server.go
💤 Files with no reviewable changes (1)
- framework/plugins/dynamicplugin.go
✅ Files skipped from review due to trivial changes (1)
- examples/plugins/hello-world-wasm-go/README.md
🚧 Files skipped from review as they are similar to previous changes (19)
- core/schemas/bifrost.go
- framework/plugins/loader.go
- examples/plugins/hello-world-wasm-typescript/assembly/tsconfig.json
- core/schemas/textcompletions.go
- examples/plugins/hello-world-wasm-rust/Cargo.toml
- plugins/jsonparser/main.go
- core/schemas/plugin_native.go
- framework/configstore/rdb.go
- core/schemas/mcp.go
- plugins/semanticcache/main.go
- .gitignore
- core/schemas/speech.go
- examples/plugins/hello-world-wasm-rust/src/memory.rs
- examples/plugins/hello-world-wasm-go/go.mod
- core/schemas/models.go
- examples/plugins/hello-world-wasm-typescript/assembly/memory.ts
- examples/plugins/hello-world-wasm-typescript/package.json
- framework/plugins/soplugin.go
- tests/integrations/config.json
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
always check the stack if there is one for the current PR. do not give localized reviews for the PR, always see all changes in the light of the whole stack of PRs (if there is a stack, if there is no stack you can continue to make localized suggestions/reviews)
Files:
core/schemas/utils.gocore/schemas/json_native.gocore/schemas/mcp_wasm.goplugins/governance/go.modcore/schemas/json_wasm.goplugins/mocker/main.goexamples/plugins/hello-world-wasm-rust/README.mdplugins/logging/main.gocore/schemas/embedding.goplugins/maxim/main.goframework/plugins/soloader.goplugins/otel/main.gocore/schemas/responses.goexamples/plugins/hello-world/main.goexamples/plugins/hello-world-wasm-typescript/Makefileexamples/plugins/hello-world-wasm-rust/Makefilecore/schemas/chatcompletions.gotransports/bifrost-http/server/server.godocs/enterprise/setting-up-okta.mdxcore/schemas/plugin.goframework/plugins/soplugin_test.goplugins/telemetry/main.goexamples/plugins/hello-world-wasm-typescript/README.mdexamples/plugins/hello-world-wasm-go/main.goframework/plugins/utils.goframework/plugins/main.gotransports/bifrost-http/lib/config.goexamples/plugins/hello-world-wasm-go/types.goplugins/governance/main.goexamples/plugins/hello-world-wasm-go/memory.goexamples/plugins/hello-world-wasm-rust/src/lib.rstransports/bifrost-http/handlers/middlewares.godocs/plugins/migration-guide.mdxdocs/plugins/writing-plugin.mdxexamples/plugins/hello-world-wasm-rust/src/types.rsexamples/plugins/hello-world-wasm-go/Makefileexamples/plugins/hello-world-wasm-typescript/assembly/index.tsexamples/plugins/hello-world-wasm-typescript/assembly/types.ts
🧠 Learnings (8)
📚 Learning: 2025-12-09T17:07:42.007Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/schemas/account.go:9-18
Timestamp: 2025-12-09T17:07:42.007Z
Learning: In core/schemas/account.go, the HuggingFaceKeyConfig field within the Key struct is currently unused and reserved for future Hugging Face inference endpoint deployments. Do not flag this field as missing from OpenAPI documentation or require its presence in the API spec until the feature is actively implemented and used. When the feature is added, update the OpenAPI docs accordingly; otherwise, treat this field as non-breaking and not part of the current API surface.
Applied to files:
core/schemas/utils.gocore/schemas/json_native.gocore/schemas/mcp_wasm.gocore/schemas/json_wasm.goplugins/mocker/main.goplugins/logging/main.gocore/schemas/embedding.goplugins/maxim/main.goframework/plugins/soloader.goplugins/otel/main.gocore/schemas/responses.goexamples/plugins/hello-world/main.gocore/schemas/chatcompletions.gotransports/bifrost-http/server/server.gocore/schemas/plugin.goframework/plugins/soplugin_test.goplugins/telemetry/main.goexamples/plugins/hello-world-wasm-go/main.goframework/plugins/utils.goframework/plugins/main.gotransports/bifrost-http/lib/config.goexamples/plugins/hello-world-wasm-go/types.goplugins/governance/main.goexamples/plugins/hello-world-wasm-go/memory.gotransports/bifrost-http/handlers/middlewares.go
📚 Learning: 2025-12-29T11:54:55.836Z
Learnt from: akshaydeo
Repo: maximhq/bifrost PR: 1153
File: framework/configstore/rdb.go:2221-2246
Timestamp: 2025-12-29T11:54:55.836Z
Learning: In Go reviews, do not flag range-over-int patterns like for i := range n as compile-time errors, assuming Go 1.22+ semantics. Only flag actual range-capable values (slices, arrays, maps, channels, strings) and other compile-time issues. This applies to all Go files across the repository.
Applied to files:
core/schemas/utils.gocore/schemas/json_native.gocore/schemas/mcp_wasm.gocore/schemas/json_wasm.goplugins/mocker/main.goplugins/logging/main.gocore/schemas/embedding.goplugins/maxim/main.goframework/plugins/soloader.goplugins/otel/main.gocore/schemas/responses.goexamples/plugins/hello-world/main.gocore/schemas/chatcompletions.gotransports/bifrost-http/server/server.gocore/schemas/plugin.goframework/plugins/soplugin_test.goplugins/telemetry/main.goexamples/plugins/hello-world-wasm-go/main.goframework/plugins/utils.goframework/plugins/main.gotransports/bifrost-http/lib/config.goexamples/plugins/hello-world-wasm-go/types.goplugins/governance/main.goexamples/plugins/hello-world-wasm-go/memory.gotransports/bifrost-http/handlers/middlewares.go
📚 Learning: 2025-12-17T08:44:08.788Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1114
File: core/schemas/chatcompletions.go:224-228
Timestamp: 2025-12-17T08:44:08.788Z
Learning: In core/schemas/chatcompletions.go, ensure the schema structures mirror OpenAI's API specifications exactly. Use the valid values for fields like ChatAudioParameters.Format and ChatAudioParameters.Voice as defined by OpenAI's documentation, and avoid adding additional inline documentation or constants to maintain direct compatibility with OpenAI's API.
Applied to files:
core/schemas/chatcompletions.go
📚 Learning: 2025-12-12T08:25:02.629Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1000
File: transports/bifrost-http/integrations/router.go:709-712
Timestamp: 2025-12-12T08:25:02.629Z
Learning: In transports/bifrost-http/**/*.go, update streaming response handling to align with OpenAI Responses API: use typed SSE events such as response.created, response.output_text.delta, response.done, etc., and do not rely on the legacy data: [DONE] termination marker. Note that data: [DONE] is only used by the older Chat Completions and Text Completions streaming APIs. Ensure parsers, writers, and tests distinguish SSE events from the [DONE] sentinel and handle each event type accordingly for correct stream termination and progress updates.
Applied to files:
transports/bifrost-http/server/server.gotransports/bifrost-http/lib/config.gotransports/bifrost-http/handlers/middlewares.go
📚 Learning: 2025-12-29T09:14:16.633Z
Learnt from: akshaydeo
Repo: maximhq/bifrost PR: 888
File: transports/bifrost-http/handlers/middlewares.go:246-256
Timestamp: 2025-12-29T09:14:16.633Z
Learning: In the bifrost HTTP transport, fasthttp.RequestCtx is the primary context carrier and should be passed directly to functions that expect a context.Context. Do not convert to context.Context unless explicitly required. Ensure tracer implementations and related components are designed to accept fasthttp.RequestCtx directly, and document this architectural decision for maintainers.
Applied to files:
transports/bifrost-http/server/server.gotransports/bifrost-http/lib/config.gotransports/bifrost-http/handlers/middlewares.go
📚 Learning: 2025-12-30T05:37:48.365Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1180
File: docs/features/mcp/connecting-to-servers.mdx:452-458
Timestamp: 2025-12-30T05:37:48.365Z
Learning: When reviewing documentation PRs in a Graphite-managed stack, first check related or previous PRs in the stack for feature implementations before flagging documentation as incorrect or unsupported. Documentation MDX files often reference features implemented in earlier stack PRs; verify that the documented behavior exists in earlier changes and that the docs accurately reflect the implemented state before requesting edits.
Applied to files:
docs/enterprise/setting-up-okta.mdxdocs/plugins/migration-guide.mdxdocs/plugins/writing-plugin.mdx
📚 Learning: 2025-12-22T10:50:40.990Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1154
File: plugins/governance/store.go:1165-1186
Timestamp: 2025-12-22T10:50:40.990Z
Learning: In the Bifrost governance plugin, budgets and rate limits have 1:1 relationships with their parent entities (virtual keys, teams, customers). Do not assume sharing; ensure cascade deletion logic only deletes budgets/rate limits when there are no shared references. Enforce invariants in code and add tests to verify no cross-entity sharing and that cascade deletes only remove the specific child of the parent. If a counterexample arises, adjust data model or add guards.
Applied to files:
plugins/governance/main.go
📚 Learning: 2026-01-07T13:31:34.442Z
Learnt from: akshaydeo
Repo: maximhq/bifrost PR: 1242
File: transports/bifrost-http/handlers/middlewares.go:109-114
Timestamp: 2026-01-07T13:31:34.442Z
Learning: The fasthttp library's Args type has an All() method with signature `func (a *Args) All() iter.Seq2[[]byte, []byte]` that returns an iterator over all key/value pairs. This method exists and should be used for iterating over query parameters or form arguments. Do not suggest using VisitAll as an alternative when All() is already being used correctly.
Applied to files:
transports/bifrost-http/handlers/middlewares.go
🧬 Code graph analysis (21)
core/schemas/utils.go (2)
core/schemas/json_native.go (1)
MarshalString(13-15)core/schemas/json_wasm.go (1)
MarshalString(13-19)
core/schemas/json_native.go (1)
core/schemas/json_wasm.go (3)
Marshal(8-10)MarshalString(13-19)Unmarshal(22-24)
core/schemas/json_wasm.go (1)
core/schemas/json_native.go (3)
Marshal(8-10)MarshalString(13-15)Unmarshal(18-20)
plugins/mocker/main.go (3)
examples/plugins/hello-world/main.go (1)
HTTPTransportIntercept(18-26)core/schemas/context.go (1)
BifrostContext(31-41)core/schemas/plugin.go (2)
HTTPRequest(38-44)HTTPResponse(48-52)
plugins/logging/main.go (3)
examples/plugins/hello-world/main.go (1)
HTTPTransportIntercept(18-26)core/schemas/context.go (1)
BifrostContext(31-41)core/schemas/plugin.go (2)
HTTPRequest(38-44)HTTPResponse(48-52)
core/schemas/embedding.go (2)
core/schemas/json_native.go (2)
Marshal(8-10)Unmarshal(18-20)core/schemas/json_wasm.go (2)
Marshal(8-10)Unmarshal(22-24)
plugins/maxim/main.go (2)
core/schemas/plugin.go (3)
Plugin(119-152)HTTPRequest(38-44)HTTPResponse(48-52)examples/plugins/hello-world/main.go (1)
HTTPTransportIntercept(18-26)
framework/plugins/soloader.go (5)
core/schemas/plugin.go (4)
Plugin(119-152)HTTPRequest(38-44)HTTPResponse(48-52)PluginShortCircuit(11-15)framework/plugins/soplugin.go (1)
DynamicPlugin(10-24)framework/plugins/utils.go (1)
DownloadPlugin(12-72)core/schemas/context.go (1)
BifrostContext(31-41)core/schemas/bifrost.go (3)
BifrostRequest(176-197)BifrostResponse(349-370)BifrostError(491-500)
plugins/otel/main.go (3)
examples/plugins/hello-world/main.go (1)
HTTPTransportIntercept(18-26)core/schemas/context.go (1)
BifrostContext(31-41)core/schemas/plugin.go (2)
HTTPRequest(38-44)HTTPResponse(48-52)
core/schemas/responses.go (2)
core/schemas/json_native.go (2)
Marshal(8-10)Unmarshal(18-20)core/schemas/json_wasm.go (2)
Marshal(8-10)Unmarshal(22-24)
examples/plugins/hello-world/main.go (3)
core/schemas/context.go (1)
BifrostContext(31-41)core/schemas/plugin.go (2)
HTTPRequest(38-44)HTTPResponse(48-52)core/schemas/bifrost.go (1)
BifrostContextKey(118-118)
core/schemas/chatcompletions.go (2)
core/schemas/json_native.go (2)
Unmarshal(18-20)Marshal(8-10)core/schemas/json_wasm.go (2)
Unmarshal(22-24)Marshal(8-10)
transports/bifrost-http/server/server.go (4)
framework/plugins/main.go (2)
LoadPlugins(22-38)Config(16-19)framework/plugins/loader.go (1)
PluginLoader(6-8)transports/bifrost-http/lib/config.go (1)
Config(212-244)framework/plugins/soloader.go (1)
SharedObjectPluginLoader(12-12)
core/schemas/plugin.go (2)
examples/plugins/hello-world/main.go (1)
HTTPTransportIntercept(18-26)core/schemas/context.go (1)
BifrostContext(31-41)
examples/plugins/hello-world-wasm-go/main.go (4)
examples/plugins/hello-world-wasm-go/types.go (6)
HTTPInterceptInput(10-13)HTTPInterceptOutput(16-22)PreHookInput(25-28)PreHookOutput(31-37)PostHookInput(40-45)PostHookOutput(48-54)core/schemas/json_native.go (2)
Unmarshal(18-20)Marshal(8-10)core/schemas/json_wasm.go (2)
Unmarshal(22-24)Marshal(8-10)plugins/mocker/main.go (1)
Response(97-103)
framework/plugins/main.go (2)
framework/plugins/loader.go (1)
PluginLoader(6-8)transports/bifrost-http/lib/config.go (1)
Config(212-244)
examples/plugins/hello-world-wasm-go/types.go (2)
core/schemas/plugin.go (3)
HTTPRequest(38-44)HTTPResponse(48-52)PluginShortCircuit(11-15)core/schemas/bifrost.go (3)
BifrostRequest(176-197)BifrostResponse(349-370)BifrostError(491-500)
plugins/governance/main.go (5)
examples/plugins/hello-world/main.go (1)
HTTPTransportIntercept(18-26)core/schemas/context.go (1)
BifrostContext(31-41)core/schemas/plugin.go (2)
HTTPRequest(38-44)HTTPResponse(48-52)core/schemas/json_native.go (2)
Unmarshal(18-20)Marshal(8-10)core/schemas/json_wasm.go (2)
Unmarshal(22-24)Marshal(8-10)
transports/bifrost-http/handlers/middlewares.go (4)
core/schemas/plugin.go (4)
AcquireHTTPRequest(67-69)ReleaseHTTPRequest(74-86)HTTPRequest(38-44)HTTPResponse(48-52)examples/plugins/hello-world/main.go (1)
HTTPTransportIntercept(18-26)core/schemas/context.go (2)
BifrostContext(31-41)NewBifrostContext(46-64)transports/bifrost-http/handlers/utils.go (1)
SendError(35-44)
examples/plugins/hello-world-wasm-typescript/assembly/index.ts (3)
examples/plugins/hello-world-wasm-rust/src/memory.rs (2)
malloc(44-53)free(61-70)examples/plugins/hello-world-wasm-typescript/assembly/memory.ts (4)
malloc(33-39)free(42-45)writeString(13-20)readString(23-30)examples/plugins/hello-world-wasm-go/types.go (6)
HTTPInterceptInput(10-13)HTTPInterceptOutput(16-22)PreHookInput(25-28)PreHookOutput(31-37)PostHookInput(40-45)PostHookOutput(48-54)
examples/plugins/hello-world-wasm-typescript/assembly/types.ts (6)
core/schemas/context.go (1)
BifrostContext(31-41)core/schemas/plugin.go (3)
HTTPRequest(38-44)HTTPResponse(48-52)PluginShortCircuit(11-15)examples/plugins/hello-world-wasm-go/types.go (4)
HTTPInterceptInput(10-13)HTTPInterceptOutput(16-22)PreHookInput(25-28)PreHookOutput(31-37)core/schemas/chatcompletions.go (4)
ChatMessage(511-520)ChatParameters(154-184)BifrostChatRequest(10-17)BifrostChatResponse(25-40)core/schemas/bifrost.go (4)
BifrostRequest(176-197)BifrostResponse(349-370)ErrorField(509-516)BifrostError(491-500)ui/lib/types/logs.ts (1)
LLMUsage(205-211)
🪛 Biome (2.1.2)
examples/plugins/hello-world-wasm-typescript/assembly/index.ts
[error] 10-10: Do not shadow the global "JSON" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
examples/plugins/hello-world-wasm-typescript/assembly/types.ts
[error] 6-6: Do not shadow the global "JSON" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🪛 checkmake (0.2.2)
examples/plugins/hello-world-wasm-typescript/Makefile
[warning] 1-1: Missing required phony target "test"
(minphony)
[warning] 16-16: Target body for "help" exceeds allowed length of 5 (10).
(maxbodylength)
[warning] 58-58: Target body for "info" exceeds allowed length of 5 (10).
(maxbodylength)
examples/plugins/hello-world-wasm-rust/Makefile
[warning] 1-1: Missing required phony target "test"
(minphony)
[warning] 17-17: Target body for "help" exceeds allowed length of 5 (10).
(maxbodylength)
[warning] 39-39: Target body for "build" exceeds allowed length of 5 (6).
(maxbodylength)
[warning] 67-67: Target body for "info" exceeds allowed length of 5 (11).
(maxbodylength)
examples/plugins/hello-world-wasm-go/Makefile
[warning] 1-1: Missing required phony target "test"
(minphony)
[warning] 20-20: Target body for "help" exceeds allowed length of 5 (11).
(maxbodylength)
[warning] 60-60: Target body for "info" exceeds allowed length of 5 (12).
(maxbodylength)
🪛 LanguageTool
examples/plugins/hello-world-wasm-rust/README.md
[style] ~524-~524: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...ies**: Rust WASM binaries are typically very small 4. Cross-platform: Single .wasm b...
(EN_WEAK_ADJECTIVE)
🪛 markdownlint-cli2 (0.18.1)
examples/plugins/hello-world-wasm-rust/README.md
48-48: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
examples/plugins/hello-world-wasm-typescript/README.md
42-42: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (59)
plugins/governance/go.mod (3)
1-128: Dependency versions across the plugin stack are consistent. Verification confirms:
- Bifrost core: v1.3.3 consistently used across all 8 plugins
- Bifrost framework: v1.2.3 consistently used across all plugins requiring it (governance, logging, maxim, otel, semanticcache, telemetry)
- Transitive dependencies: No conflicts detected—fasthttp v1.68.0 and sonic v1.14.2 are uniformly versioned across all plugins
The governance module is properly aligned with the stack (PRs #1266, #1262, #1242).
104-104: Fasthttp relocation to indirect dependency is safe.The governance plugin code does not directly import or use fasthttp APIs, and the transitive dependency remains stable through the core module which has fasthttp as a direct dependency. No runtime errors are expected from this change.
8-8: Remove this comment—sonic is actively used in governance plugin.The governance plugin actively imports and uses
github.com/bytedance/sonicfor JSON marshaling/unmarshaling (e.g.,sonic.Unmarshal(req.Body, &payload)andsonic.Marshal(payload)). The direct dependency at line 8 is intentional and necessary. The enriched summary's mention of sonic removal applies to other modules, not governance.Likely an incorrect or invalid review comment.
examples/plugins/hello-world-wasm-go/memory.go (4)
9-29: LGTM! Fixed-size heap addresses previous concerns.The fixed-size heap approach correctly addresses the dangling pointer issue raised in previous reviews. Pre-allocating the heap once in
init()and cachingheapBasePtrensures pointer stability throughout the plugin's lifetime.
68-82: Good fix: writeBytes now uses plugin_malloc.The implementation correctly addresses previous feedback by using
plugin_mallocinstead of Go's managed heap, ensuring consistent memory management through the custom allocator.Note: The pointer arithmetic at line 79 (
offset := ptr - uint32(heapBasePtr)) assumes 32-bit address space. This is correct for WASM targets but would break if this code were compiled as native Go on 64-bit platforms. Given the file location (examples/plugins/hello-world-wasm-go/), this is appropriate for the WASM-only context.
84-89: LGTM! Appropriate use of unsafe.Slice for WASM interop.The implementation correctly uses
unsafe.Slice(Go 1.17+) to create a slice from a raw pointer and length, which is the idiomatic approach for WASM plugin interop. The zero-length check is appropriate.
56-62: This function is unused in the Bifrost host implementation. While the function itself is correctly documented,plugin_resetis not part of the documented plugin interface (which specifiesplugin_malloc,plugin_free,get_name,http_transport_intercept,pre_hook,post_hook,cleanup, andinit), and no host code calls it. The other WASM plugin examples (Rust and TypeScript) do not export this function either.Consider removing
plugin_resetunless it's explicitly needed for future requirements. If it's meant to be a stable part of the plugin contract, document it in the README alongside the other required exports.core/schemas/utils.go (1)
268-268: LGTM! Clean abstraction for cross-platform JSON serialization.The change from
sonic.MarshalStringtoMarshalStringproperly delegates to build-constrained wrappers (sonic for native, encoding/json for WASM/TinyGo). This maintains compatibility while enabling WASM plugin support.plugins/maxim/main.go (1)
159-162: LGTM! Correct migration to the new HTTPTransportIntercept API.The implementation correctly returns
(nil, nil)to indicate that this plugin doesn't need HTTP transport interception, allowing processing to continue unchanged. This aligns with the documented intercept pattern across the plugin ecosystem.core/schemas/json_wasm.go (1)
1-24: LGTM! Well-implemented WASM/TinyGo JSON wrappers.The build-constrained wrappers correctly abstract JSON operations for WASM/TinyGo environments using
encoding/json. The API maintains parity withjson_native.go, ensuring consistent behavior across build targets while enabling WASM plugin support.examples/plugins/hello-world-wasm-typescript/Makefile (1)
1-70: LGTM! Well-structured Makefile for TypeScript WASM plugin.The Makefile provides a clean build pipeline with appropriate dependency checks, multiple build modes (release and debug), and helpful info/help targets. The structure aligns well with the Go WASM example while using Node.js/npm tooling appropriately.
core/schemas/json_native.go (1)
1-20: LGTM!Clean implementation of build-tagged JSON wrappers. The API matches
json_wasm.gocounterpart, enabling seamless switching between high-performance sonic (native) and standard library (WASM/TinyGo) based on build target.examples/plugins/hello-world-wasm-rust/Makefile (1)
1-80: LGTM!Well-structured Makefile with good developer experience features: colorized output, prerequisite checks, optional optimization, and helpful
infotarget. The static analysis warnings about missingtesttarget and body lengths are acceptable for an example plugin - this is demonstrational scaffolding, not production code requiring strict linting.core/schemas/plugin.go (3)
35-52: LGTM - Serializable HTTP types for cross-platform plugin support.The
HTTPRequestandHTTPResponsestructs withmap[string]stringfor headers/query and[]bytefor body provide a clean, serializable interface that works across native.soand WASM plugins. This is a good design choice for the interception API.
119-135: Clear API contract for HTTP transport interception.The documentation clearly specifies the return value semantics:
(nil, nil)→ continue processing(*HTTPResponse, nil)→ short-circuit with response(nil, error)→ short-circuit with errorThis aligns with the example in
/examples/plugins/hello-world/main.go.
54-86: Well-designed pooling implementation.The
sync.Poolusage with pre-allocated map capacities (16 for headers, 8 for query) is a good optimization. TheReleaseHTTPRequestproperly clears maps and resets all fields before returning to pool. The project's Go version requirement (1.25.5) is compatible with theclear()builtin, which was introduced in Go 1.21.examples/plugins/hello-world-wasm-typescript/assembly/types.ts (2)
6-6: Intentional import shadowing - ignore static analysis warning.The
JSONimport fromjson-asis required for the@jsondecorator functionality in AssemblyScript. This shadowing of the globalJSONis intentional and necessary for the serialization framework.
18-26: BifrostContext is intentionally simplified for WASM boundary.The TypeScript
BifrostContextonly includesrequest_idand custom plugin fields, whereas the GoBifrostContexthas deadline, done channel, error mutex, etc. This simplification is acceptable since the full context management happens on the Go side, and only serializable values need to cross the WASM boundary.core/schemas/embedding.go (1)
1-164: LGTM - Clean refactor to use abstracted JSON wrappers.All
sonic.Marshal/sonic.Unmarshalcalls are replaced with the package-localMarshal/Unmarshalwrappers. This aligns with the broader refactor across schema files, enabling build-tagged switching between sonic (native) and standard library (WASM/TinyGo). No logic changes, just consistent abstraction.plugins/telemetry/main.go (1)
279-282: LGTM! Correct migration to HTTPTransportIntercept.The implementation correctly adopts the new HTTP transport interception pattern by returning
(nil, nil)to indicate that this plugin does not intercept HTTP transport layer requests. This aligns with the broader migration from middleware-based to intercept-based plugin integration visible across the PR.plugins/mocker/main.go (1)
481-484: LGTM! Consistent migration pattern.The HTTPTransportIntercept implementation is correct and consistent with the migration pattern applied across all plugins in this PR.
plugins/otel/main.go (1)
148-151: LGTM! Consistent with migration pattern.The HTTPTransportIntercept implementation correctly indicates that this plugin does not require HTTP transport layer interception.
transports/bifrost-http/server/server.go (2)
223-223: LGTM! Correct integration with new PluginLoader abstraction.The call to
LoadPluginsnow correctly passesbifrostConfig.PluginLoaderas the first argument, aligning with the new plugin loading contract. The loader is properly initialized at line 1198 in the Bootstrap method before this code path is executed.
1197-1198: LGTM! Correct PluginLoader initialization.The initialization of
PluginLoaderwithSharedObjectPluginLoaderis correct and enables dynamic plugin loading functionality. This is properly placed in the Bootstrap method before any plugin loading operations occur.plugins/logging/main.go (1)
194-197: LGTM! Consistent migration to new intercept pattern.The HTTPTransportIntercept implementation is correct and consistent with the migration pattern applied uniformly across all plugins in this PR.
examples/plugins/hello-world/main.go (1)
18-26: LGTM! Clean migration to the new intercept pattern.The function correctly demonstrates the new
HTTPTransportInterceptsignature and behavior:
- In-place request modification via
req.Headers- Context value storage for downstream hooks
- Clear semantics:
(nil, nil)continues processing, non-nilHTTPResponseshort-circuitsThis aligns with the broader plugin transport refactor described in the PR and migration guide.
core/schemas/responses.go (1)
156-207: LGTM! Systematic JSON library migration.The refactor consistently replaces
sonic.Marshal/sonic.Unmarshalwith genericMarshal/Unmarshalwrappers throughout the file. This enables:
- Native builds: delegated to
sonicviajson_native.go- WASM/TinyGo builds: delegated to
encoding/jsonviajson_wasm.goThe pattern is uniform, error handling is preserved, and no public APIs are affected.
core/schemas/chatcompletions.go (1)
201-229: LGTM! Consistent JSON library migration.This file follows the same systematic refactor pattern as
responses.go, replacing directsoniccalls with genericMarshal/Unmarshalwrappers to enable WASM/TinyGo support via build constraints. Error handling and control flow are preserved.transports/bifrost-http/lib/config.go (3)
28-28: LGTM!The addition of the plugins import aligns with the new PluginLoader field introduced in this PR.
237-237: LGTM!The PluginLoader field properly integrates the new plugin loading abstraction into the Config struct, enabling dynamic plugin loading via the loader interface.
478-564: LGTM!The hash-based reconciliation logic is well-structured with clear separation of three cases:
- No DB config - use file/defaults and generate hash
- Only DB config - use as-is with backward compatibility for zero values
- Both configs - compare hashes and sync from file on mismatch
This approach ensures file changes are detected and applied while preserving UI-driven changes when hashes match.
framework/plugins/soloader.go (2)
34-51: LGTM!The optional Init symbol lookup correctly handles the case where Init is not present in the plugin, while properly invoking it with the provided config when present.
53-93: LGTM!The symbol lookups for GetName, HTTPTransportIntercept, PreHook, PostHook, and Cleanup all use proper type assertions with exact function signatures matching the Plugin interface. Error handling is appropriate for each lookup.
framework/plugins/soplugin_test.go (2)
41-42: LGTM!The test correctly instantiates a SharedObjectPluginLoader and uses the new two-argument LoadPlugins signature, aligning with the loader-based plugin loading pattern introduced in this PR.
55-79: LGTM!The HTTPTransportIntercept test properly exercises the new interception API:
- Creates a serialized HTTPRequest with headers, body, and other fields
- Invokes HTTPTransportIntercept directly instead of using middleware
- Verifies in-place header modifications on the request object
- Confirms nil response is returned to continue processing
docs/plugins/migration-guide.mdx (3)
9-40: LGTM!The overview and key differences section clearly explains the migration from v1.3.x to v1.4.x, accurately documenting:
- The shift from TransportInterceptor to HTTPTransportIntercept
- Unified HTTPRequest struct vs separate headers/body maps
- In-place modification pattern
- WASM support and improved context access
The comparison table effectively highlights the API changes.
42-224: LGTM!The migration steps and common patterns are comprehensive and accurate:
- Correctly shows removal of fasthttp imports
- Demonstrates in-place modifications using req.Headers and req.Body
- Proper short-circuit examples returning *HTTPResponse
- Accurate body modification patterns using sonic for marshal/unmarshal
- New request method/path access patterns
The code examples align with the implementation changes observed in the codebase.
226-292: LGTM!The testing and troubleshooting section provides practical guidance:
- Build and test commands
- Log verification examples
- Common error resolutions for symbol not found, body modification, and header issues
- Clear explanations of correct vs incorrect patterns
docs/plugins/writing-plugin.mdx (2)
89-98: LGTM! Clear documentation of the new HTTPTransportIntercept API.The function signature and in-place modification semantics are well documented. The example correctly demonstrates header modification and the return convention for pass-through behavior.
211-228: LGTM! Comprehensive explanation of the intercept model.The key points section clearly explains the return value semantics and the difference from the old TransportInterceptor approach. The warning about bifrost-http vs SDK usage is helpful.
examples/plugins/hello-world-wasm-go/main.go (2)
17-30: LGTM! Plugin exports correctly defined.The
get_nameandinit_pluginexports follow the expected WASM plugin contract. The//export initdirective correctly maps to the exported symbol name while usinginit_pluginas the Go function name to avoid conflicts with Go's specialinitfunction.
66-101: LGTM! Pre-hook implementation follows the expected pattern.Context augmentation and pass-through behavior are correctly implemented. The error handling paths use dedicated helper functions.
examples/plugins/hello-world-wasm-rust/src/lib.rs (3)
70-76: LGTM! Request is now correctly preserved.The
http_interceptoutput now properly passes throughinput.requestinstead of setting it to an empty JSON object. This addresses the previous review feedback.
115-121: LGTM! Pre-hook request preservation fixed.The
pre_hookoutput now correctly clonesinput.requestfor pass-through, addressing the previous review feedback.
14-15: Acceptable use ofstatic mutfor WASM context.The
unsafeglobal state is acceptable here since WASM modules are single-threaded. For production plugins with more complex state, consider usingOnceCellor similar patterns for cleaner initialization.Also applies to: 44-46
plugins/governance/main.go (2)
262-303: LGTM! Clean migration to the new HTTPTransportIntercept API.The governance plugin correctly:
- Mutates
req.Headersin-place for MCP tools- Unmarshal/marshal the body for load balancing modifications
- Returns
(nil, nil)to continue processing (not short-circuiting)- Handles errors gracefully by logging and continuing
232-260: LGTM! Header parsing correctly adapted to HTTPRequest structure.The
parseVirtualKeyFromHTTPRequestfunction correctly reads headers from thereq.Headersmap instead of the previous fasthttp context. The virtual key detection logic is preserved.examples/plugins/hello-world-wasm-go/types.go (1)
1-54: LGTM! Well-structured type definitions for WASM plugin contract.The input/output structs correctly define:
- Context as a flexible
map[string]interface{}for cross-hook data sharing- Has* boolean flags to signal presence of optional data (HasResponse, HasShortCircuit, HasError)
- Separate error fields for hook-level errors vs propagated errors
- Proper JSON tags with
omitemptyfor optional fieldsThese types align with the core schema definitions and provide a clean contract for WASM plugins.
examples/plugins/hello-world-wasm-typescript/assembly/index.ts (4)
10-10: JSON import shadowing is intentional for AssemblyScript.The static analysis tool flags this as shadowing the global
JSON, but in AssemblyScript, thejson-aslibrary provides the JSON serialization functionality since AssemblyScript doesn't have a built-inJSONglobal. This is the standard pattern for AssemblyScript projects.
70-93: LGTM! HTTP intercept follows the expected pattern.The implementation correctly:
- Parses input using json-as
- Preserves context in output
- Sets
has_response = falsefor pass-through- Includes commented example for short-circuit behavior
100-168: LGTM! Pre-hook with context augmentation.Context values are correctly set for tracking (
plugin_processed,plugin_name). The provider/model extraction handles both direct fields and nestedchat_requeststructure.
175-228: LGTM! Post-hook correctly handles success and error paths.The implementation properly:
- Checks
plugin_processedfrom context- Propagates errors when
has_erroris true- Passes through the response on success
examples/plugins/hello-world-wasm-rust/src/types.rs (4)
50-69: Note: Body fields use base64-encoded strings.The
HTTPRequestandHTTPResponsestructs definebodyasString(base64-encoded) rather than raw bytes. This is documented in the comments and is appropriate for JSON serialization across the WASM boundary.Also applies to: 72-83
421-450: LGTM! Clean builder pattern for BifrostError.The fluent API with
new(),with_type(),with_code(), andwith_status()provides an ergonomic way to construct errors in plugin code.
587-640: LGTM! Good test coverage for type serialization.Tests cover:
- Context serialization with custom values
- Chat message structure
- Error builder pattern
- Pre-hook input parsing with provider/model extraction
1-581: LGTM! Comprehensive type definitions for Rust WASM plugins.The types correctly mirror the Go SDK structures with appropriate serde annotations, default values, and helper methods. The
#[serde(flatten)]usage for extra fields provides extensibility without breaking compatibility.docs/enterprise/setting-up-okta.mdx (1)
1-322: Verify file inclusion in this PR.This file contains Okta SSO setup documentation, which doesn't appear to relate to the PR's stated objectives about plugin loaders, File APIs, and WASM plugins. While the retrieved learnings note that documentation in a Graphite-managed stack may reference features from other PRs in the stack, this entire file seems unrelated to plugin functionality.
Question: Is this file intentionally included in this PR, or should it be part of a different PR in the stack (#1266, #1262)?
Based on learnings about Graphite-managed stacks.
core/schemas/mcp_wasm.go (2)
1-2: Build constraint looks good.The build constraint correctly limits this file to TinyGo/WASM targets, which aligns with the documented purpose of providing a stub for WASM plugins.
5-7: The empty struct stub is appropriate for WASM builds.The stub is sufficient because WASM plugins only import
core/schemasand do not include thetransportspackage. Field accesses toMCPConfig.ClientConfigsandMCPConfig.ToolManagerConfigoccur exclusively intransports/bifrost-http/lib/config.go, which is not compiled for WASM. The empty struct provides the necessary type compatibility without requiring field or method stubs.
511a030 to
6272071
Compare
Merge activity
|

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 guidelines