Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughIntroduces route-based plugin invocation with wildcard path support, GET method handling, raw response mode, and per-invocation configuration. These capabilities propagate through the HTTP API layer down to the TypeScript plugin runtime, enabling plugins to receive routing context, HTTP method, query parameters, and custom configuration via an extended PluginContext. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Router as HTTP Router
participant Controller as Plugin Controller
participant Service as Plugin Service
participant Runner as Plugin Runner
participant Executor as Script Executor
participant TypeScript as TypeScript Runtime
Client->>Router: POST/GET /api/v1/plugins/{id}/call/route
Router->>Router: Extract route, query params, headers
Router->>Controller: PluginCallRequest{route, method, query, ...}
alt GET invocation
Router->>Router: Validate allow_get_invocation
Note over Router: 405 if disabled
end
Controller->>Service: call_plugin(request)
Service->>Service: Serialize route, method, query, config to JSON
Service->>Runner: run(..., route, method, query_json, config_json)
Runner->>Executor: execute_typescript(..., route, method, query_json, config_json)
Executor->>TypeScript: Invoke with argv[8-11]: route, method, query, config
TypeScript->>TypeScript: Parse CLI args into PluginContext{route, method, query, config}
TypeScript->>TypeScript: Execute handler(context)
alt raw_response enabled
TypeScript-->>Executor: Plugin result (raw)
Executor-->>Runner: Result
Runner-->>Service: Result
Service-->>Controller: Result
Controller-->>Client: HTTP response (raw body)
else standard response
TypeScript-->>Executor: Plugin result
Executor-->>Runner: Result
Runner-->>Service: Result
Service-->>Controller: Result (wrapped in ApiResponse)
Controller-->>Client: HTTP response (ApiResponse envelope)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (3)
plugins/lib/executor.ts (1)
35-167: Fix exit/cleanup flow:process.exit(1)incatchcan skipfinallycleanup; use a single exit path.async function main(): Promise<void> { const logInterceptor = new LogInterceptor(); + let exitCode = 0; try { // Start intercepting all console output at the executor level // This provides better backward compatibility with existing scripts logInterceptor.start(); @@ const result = await runUserPlugin(socketPath, pluginId, pluginParams, userScriptPath, httpRequestId, headers, route, config, method, query); // Add the result to LogInterceptor output logInterceptor.addResult(serializeResult(result)); } catch (error) { process.stderr.write(`Plugin executor failed: ${error instanceof Error ? error.message : error}\n`); - process.exit(1); + exitCode = 1; } finally { logInterceptor.stop(); - process.exit(0); + process.exit(exitCode); } }plugins/lib/plugin.ts (2)
206-215: Harden modern-vs-legacy handler detection:handler.length === 1is brittle (default params/rest).if (handler && typeof handler === 'function') { // Detect handler signature by parameter count - if (handler.length === 1) { + // Treat only the classic `(api, params)` as legacy; everything else gets context. + if (handler.length !== 2) { // Modern context handler - ONLY these get KV, headers, config, method, and query access const context: PluginContext = { api, params, kv, headers: headers ?? {}, route: route ?? '', config, method: method ?? 'POST', query: query ?? {} }; return await handler(context); } else { // Legacy handler - NO KV or headers access, just (api, params) // This keeps PluginAPI interface unchanged return await handler(api, params); } }Also applies to: 299-386
290-309: Update JSDoc for newconfig/method/queryparams to match the function signatures.Also applies to: 543-568
🧹 Nitpick comments (9)
src/config/config_file/plugin.rs (1)
5-5:serde_json::Mapimport is fine; consider aliasing for clarity if used widely.If this pattern appears in multiple config structs,
type JsonObject = serde_json::Map<String, serde_json::Value>;can reduce repetition.src/api/routes/docs/plugin_docs.rs (1)
122-199: GET invocation docs are good; consider notingraw_responseaffects envelope shape.Right now the 200 response is always
ApiResponse<serde_json::Value>, but plugins configured withraw_response: truemay return an unwrapped body—worth calling out (and/or using aoneOfschema if feasible with utoipa).src/api/controllers/plugin.rs (1)
120-124: Consider raw_response handling for Fatal errors.The
Fatalerror path always returns a wrappedApiResponse, unlikeSuccessandHandlerpaths which respect theraw_responseflag. If this is intentional (to always wrap internal errors), consider adding a comment explaining the rationale. Otherwise, consider applying the same conditional logic for consistency.src/models/plugin.rs (2)
32-49: Avoid exposing injected fields in request-body OpenAPI schema (or mark them read-only).
headers/route/method/queryare server-injected (skip_deserializing), butToSchemawill still document them as client-provided unless you explicitly hide/mark them.Suggested approach: introduce a separate
PluginCallBody { params: Value }(and use that for OpenAPI / request parsing), or annotate these with#[schema(read_only)]if you still want them visible as “server-set”.
22-30: Document/encode the “config must be a JSON object” constraint in the type/schema.
config: Option<Map<String, Value>>enforces object-ness at deserialization time (good), but the API schema may still be vague. Consider adding#[serde(default, skip_serializing_if = "Option::is_none")]and a#[schema(...)]hint for clearer OpenAPI. (As per coding guidelines / learnings: serde-derived JSON types are preferred.)src/services/plugins/mod.rs (1)
232-243: Don’t useunwrap_or_default()for JSON that will be parsed later (can generate invalid JSON).
If serialization ever fails, passing""risks downstream parse errors and confusing behavior.Consider returning
PluginCallResult::Fatal(PluginError::InvalidPayload(...))on serialization errors, or at leastexpect(...)with a clear message.- let headers_json = plugin_call_request - .headers - .map(|h| serde_json::to_string(&h).unwrap_or_default()); + let headers_json = match plugin_call_request.headers { + Some(h) => match serde_json::to_string(&h) { + Ok(s) => Some(s), + Err(e) => { + return PluginCallResult::Fatal(PluginError::InvalidPayload(format!( + "Failed to serialize headers: {e}" + ))) + } + }, + None => None, + };Apply the same pattern to
config_jsonandquery_json.Also applies to: 244-260
src/api/routes/plugin.rs (1)
66-120: Consider allowing empty POST bodies (treat as{}) if “no params” calls are valid.
Right nowserde_json::from_slice(&body)will 400 on an empty body. If the intended contract allows calling a plugin with no params, consider treating empty as{}(ornull) before parsing.openapi.json (2)
747-748: POST call docs: “route = ""” vs examples should clarify leading slash semantics.
The description mixesroute = ""androute = "/verify"; please ensure clients can rely on one convention (empty vs/-prefixed), and consider explicitly stating whethercontext.routeis always"" | ("/" + path).
6317-6367:PluginModellooks good; consider marking optional booleans as optional-with-default in docs.
Schema shape is clear and matches the feature set (allow_get_invocation,emit_logs,emit_traces,raw_response,config) (Line 6317-6367). If the server applies defaults (e.g.,false), addingdefault: falsecan reduce client ambiguity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
examples/x402-facilitator-plugin-example/x402-facilitator/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlplugins/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (26)
docs/guides/stellar-x402-facilitator-guide.mdx(1 hunks)docs/plugins/index.mdx(6 hunks)examples/x402-facilitator-plugin-example/.env.example(1 hunks)examples/x402-facilitator-plugin-example/README.md(1 hunks)examples/x402-facilitator-plugin-example/config/config.json(1 hunks)examples/x402-facilitator-plugin-example/docker-compose.yaml(1 hunks)examples/x402-facilitator-plugin-example/x402-facilitator/index.ts(1 hunks)examples/x402-facilitator-plugin-example/x402-facilitator/package.json(1 hunks)examples/x402-facilitator-plugin-example/x402-facilitator/tsconfig.json(1 hunks)openapi.json(4 hunks)plugins/lib/executor.ts(4 hunks)plugins/lib/plugin.ts(4 hunks)src/api/controllers/plugin.rs(8 hunks)src/api/routes/docs/plugin_docs.rs(2 hunks)src/api/routes/plugin.rs(7 hunks)src/bootstrap/config_processor.rs(2 hunks)src/config/config_file/mod.rs(2 hunks)src/config/config_file/plugin.rs(2 hunks)src/models/plugin.rs(2 hunks)src/openapi.rs(1 hunks)src/repositories/plugin/mod.rs(4 hunks)src/repositories/plugin/plugin_in_memory.rs(8 hunks)src/repositories/plugin/plugin_redis.rs(1 hunks)src/services/plugins/mod.rs(23 hunks)src/services/plugins/runner.rs(5 hunks)src/services/plugins/script_executor.rs(9 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (.cursor/rules/rust_standards.mdc)
**/*.rs: Follow official Rust style guidelines using rustfmt (edition = 2021, max_width = 100)
Follow Rust naming conventions: snake_case for functions/variables, PascalCase for types, SCREAMING_SNAKE_CASE for constants and statics
Order imports alphabetically and group by: std, external crates, local crates
Include relevant doc comments (///) on public functions, structs, and modules. Use comments for 'why', not 'what'. Avoid redundant doc comments
Document lifetime parameters when they're not obvious
Avoid unsafe code unless absolutely necessary; justify its use in comments
Write idiomatic Rust: Prefer Result over panic, use ? operator for error propagation
Avoid unwrap; handle errors explicitly with Result and custom Error types for all async operations
Prefer header imports over function-level imports of dependencies
Prefer borrowing over cloning when possible
Use &str instead of &String for function parameters
Use &[T] instead of &Vec for function parameters
Avoid unnecessary .clone() calls
Use Vec::with_capacity() when size is known in advance
Use explicit lifetimes only when necessary; infer where possible
Always use Tokio runtime for async code. Await futures eagerly; avoid blocking calls in async contexts
Streams: Use futures::StreamExt for processing streams efficiently
Always use serde for JSON serialization/deserialization with #[derive(Serialize, Deserialize)]
Use type aliases for complex types to improve readability
Implement common traits (Debug, Clone, PartialEq) where appropriate, using derive macros when possible
Implement Display for user-facing types
Prefer defining traits when implementing services to make mocking and testing easier
For tests, prefer existing utils for creating mocks instead of duplicating code
Use assert_eq! and assert! macros appropriately
Keep tests minimal, deterministic, and fast
Use tracing for structured logging, e.g., tracing::info! for request and error logs
When optimizing, prefer clarity first, then performance
M...
Files:
src/repositories/plugin/plugin_redis.rssrc/openapi.rssrc/config/config_file/plugin.rssrc/services/plugins/runner.rssrc/config/config_file/mod.rssrc/models/plugin.rssrc/bootstrap/config_processor.rssrc/services/plugins/script_executor.rssrc/repositories/plugin/mod.rssrc/api/controllers/plugin.rssrc/api/routes/docs/plugin_docs.rssrc/repositories/plugin/plugin_in_memory.rssrc/services/plugins/mod.rssrc/api/routes/plugin.rs
🧠 Learnings (2)
📚 Learning: 2025-12-02T11:22:31.387Z
Learnt from: CR
Repo: OpenZeppelin/openzeppelin-relayer PR: 0
File: .cursor/rules/rust_standards.mdc:0-0
Timestamp: 2025-12-02T11:22:31.387Z
Learning: Applies to **/*.rs : Always use serde for JSON serialization/deserialization with #[derive(Serialize, Deserialize)]
Applied to files:
src/config/config_file/plugin.rssrc/models/plugin.rs
📚 Learning: 2025-12-02T11:22:31.387Z
Learnt from: CR
Repo: OpenZeppelin/openzeppelin-relayer PR: 0
File: .cursor/rules/rust_standards.mdc:0-0
Timestamp: 2025-12-02T11:22:31.387Z
Learning: Applies to **/*.rs : Include relevant doc comments (///) on public functions, structs, and modules. Use comments for 'why', not 'what'. Avoid redundant doc comments
Applied to files:
src/api/routes/docs/plugin_docs.rs
🧬 Code graph analysis (10)
src/openapi.rs (1)
src/api/routes/docs/plugin_docs.rs (2)
doc_call_plugin_get(202-202)doc_list_plugins(276-276)
src/config/config_file/plugin.rs (3)
src/api/routes/plugin.rs (1)
serde_json(89-89)src/services/plugins/mod.rs (1)
serde_json(281-281)src/services/plugins/script_executor.rs (1)
serde_json(100-100)
src/config/config_file/mod.rs (2)
src/models/network/repository.rs (1)
config(136-138)src/domain/transaction/evm/utils.rs (3)
None(293-293)None(324-324)None(356-356)
plugins/lib/executor.ts (1)
plugins/lib/plugin.ts (1)
runUserPlugin(557-589)
src/models/plugin.rs (3)
src/api/routes/plugin.rs (1)
serde_json(89-89)src/services/plugins/mod.rs (1)
serde_json(281-281)src/services/plugins/script_executor.rs (1)
serde_json(100-100)
plugins/lib/plugin.ts (1)
plugins/lib/kv.ts (2)
PluginKVStore(28-88)DefaultPluginKVStore(96-277)
src/api/routes/docs/plugin_docs.rs (3)
src/api/routes/plugin.rs (1)
serde_json(89-89)src/services/plugins/mod.rs (1)
serde_json(281-281)src/services/plugins/script_executor.rs (1)
serde_json(100-100)
src/repositories/plugin/plugin_in_memory.rs (1)
src/models/network/repository.rs (1)
config(136-138)
src/services/plugins/mod.rs (2)
src/api/routes/plugin.rs (1)
serde_json(89-89)src/services/plugins/script_executor.rs (1)
serde_json(100-100)
src/api/routes/plugin.rs (2)
src/repositories/plugin/plugin_in_memory.rs (1)
new(36-40)src/models/api_response.rs (2)
new(37-45)error(57-65)
🪛 dotenv-linter (4.0.0)
examples/x402-facilitator-plugin-example/.env.example
[warning] 2-2: [UnorderedKey] The API_KEY key should go before the REDIS_URL key
(UnorderedKey)
[warning] 3-3: [UnorderedKey] The KEYSTORE_PASSPHRASE key should go before the REDIS_URL key
(UnorderedKey)
🪛 Gitleaks (8.30.0)
examples/x402-facilitator-plugin-example/README.md
[high] 184-185: Discovered a potential authorization token provided in a curl command header, which could compromise the curl accessed resource.
(curl-auth-header)
docs/guides/stellar-x402-facilitator-guide.mdx
[high] 219-220: Discovered a potential authorization token provided in a curl command header, which could compromise the curl accessed resource.
(curl-auth-header)
[high] 263-264: Discovered a potential authorization token provided in a curl command header, which could compromise the curl accessed resource.
(curl-auth-header)
[high] 300-301: Discovered a potential authorization token provided in a curl command header, which could compromise the curl accessed resource.
(curl-auth-header)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: msrv
- GitHub Check: test
- GitHub Check: clippy
- GitHub Check: boostsecurity - boostsecurityio/semgrep-pro
- GitHub Check: Redirect rules - openzeppelin-relayer
- GitHub Check: Header rules - openzeppelin-relayer
- GitHub Check: Pages changed - openzeppelin-relayer
- GitHub Check: Analyze (rust)
- GitHub Check: semgrep/ci
🔇 Additional comments (25)
examples/x402-facilitator-plugin-example/x402-facilitator/tsconfig.json (1)
1-13: TS config looks reasonable for a Node/CommonJS example.examples/x402-facilitator-plugin-example/x402-facilitator/index.ts (1)
1-1: Clean entrypoint export for the plugin handler.examples/x402-facilitator-plugin-example/README.md (1)
183-186: Avoid gitleaks false-positive by using an env var in the curl Authorization header.-curl -X GET http://localhost:8080/api/v1/relayers/stellar-example \ - -H "Authorization: Bearer YOUR_API_KEY" +curl -X GET http://localhost:8080/api/v1/relayers/stellar-example \ + -H "Authorization: Bearer ${API_KEY}"⛔ Skipped due to learnings
Learnt from: sammccord Repo: OpenZeppelin/openzeppelin-relayer PR: 457 File: src/models/signer/request.rs:885-971 Timestamp: 2025-09-11T22:35:15.583Z Learning: In test files for OpenZeppelin Relayer, base64-encoded test secrets (like "dGVzdC1hcGkta2V5LXNlY3JldA==" which decodes to "test-api-key-secret") should be kept in their base64 format to properly test validation logic, even if they trigger gitleaks false positives. These test fixtures with obviously fake values are acceptable exceptions to secret detection rules.src/config/config_file/plugin.rs (1)
22-29: New plugin fields + serde defaults look correct.
#[serde(default)]on the bools gives backward-compatible config loading (missing =>false).config: Option<Map<...>>cleanly constrains the value to “JSON object” shape.src/repositories/plugin/plugin_redis.rs (1)
334-345: Test fixture updated appropriately for newPluginModelfields.src/openapi.rs (1)
84-87: Good: new plugin endpoints are included in OpenAPI generation.Please confirm the repo’s
utoipaversion and that the generated spec includes:
GET /api/v1/pluginsGET /api/v1/plugins/{plugin_id}/callsrc/config/config_file/mod.rs (2)
241-250: Test config updated correctly for new plugin fields.
1150-1159: Invalid-extension test updated correctly for new plugin fields.src/bootstrap/config_processor.rs (2)
1064-1088: Plugin fixtures updated appropriately for new config fields.
1193-1202: End-to-end config processing test updated appropriately for new plugin fields.examples/x402-facilitator-plugin-example/config/config.json (1)
1-54: Example config looks consistent with the new plugin config schema.src/services/plugins/script_executor.rs (4)
41-53: LGTM on the extended signature with new context parameters.The new optional parameters (
route,config_json,method,query_json) are well-organized and follow the existing pattern for optional parameters. Using#[allow(clippy::too_many_arguments)]is acceptable here given the function's role as a CLI argument bridge.
79-82: Arguments are correctly forwarded to the TypeScript executor.The new parameters are properly appended as positional arguments with clear comments indicating their argv indices. Using
unwrap_or_default()ensures empty strings are passed when values are absent, which aligns with the existing pattern.
477-529: Good test coverage for route parameter handling.The test properly validates that the route parameter flows through to the TypeScript context and is accessible within the handler. The assertion pattern is consistent with other tests in this file.
215-218: Test updates are consistent and complete.All existing tests have been properly updated with
Noneplaceholders for the new optional parameters, maintaining backward compatibility while enabling the new functionality.src/repositories/plugin/plugin_in_memory.rs (2)
131-134: Test fixtures properly extended with new PluginModel fields.The new fields
raw_response,allow_get_invocation, andconfigare consistently initialized across all test cases with appropriate default values (false,false,None).
151-177: TryFrom conversion test properly validates new field propagation.The test correctly verifies that the new fields (
raw_response,allow_get_invocation,config) are properly converted fromPluginFileConfigtoPluginModel.src/repositories/plugin/mod.rs (2)
141-144: TryFrom implementation cleanly propagates new fields.The new fields are correctly transferred from
PluginFileConfigtoPluginModel. The implementation follows the existing pattern of direct field mapping.
191-202: Test helper function properly updated.The
create_test_pluginhelper maintains DRY principles by providing consistent default values for the new fields across tests.src/api/controllers/plugin.rs (3)
60-60: Raw response mode implemented correctly for success path.The conditional logic cleanly separates raw responses from wrapped
ApiResponseobjects. Whenraw_responseis true, the plugin result is returned directly without metadata wrapper.Also applies to: 69-77
110-118: Handler error path correctly respects raw_response flag.The error handling maintains consistent behavior with the success path - raw responses return the error object directly while wrapped responses include additional metadata.
323-354: Test validates raw_response code path execution.While the test can't fully validate the response body due to ts-node unavailability in the test environment, it correctly verifies that the
raw_responseflag is being read and processed without causing errors.src/services/plugins/runner.rs (3)
44-47: Trait and implementation correctly extended with new context parameters.The
PluginRunnerTraitandPluginRunnerimplementation are properly updated to accept and forward the new parameters (route,config_json,method,query_json) to the script execution layer.Also applies to: 80-83
111-124: Parameters correctly forwarded to ScriptExecutor.The new parameters are passed in the correct order matching the
ScriptExecutor::execute_typescriptsignature, maintaining consistency across the execution chain.
222-226: Tests properly updated with new parameter placeholders.Both
test_runandtest_run_timeouttests are correctly updated withNonevalues for the new optional parameters, maintaining existing test coverage.
Codecov Report❌ Patch coverage is
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. @@ Coverage Diff @@
## main #587 +/- ##
==========================================
- Coverage 92.30% 92.29% -0.02%
==========================================
Files 250 250
Lines 90865 91207 +342
==========================================
+ Hits 83875 84178 +303
- Misses 6990 7029 +39
🚀 New features to boost your workflow:
|
zeljkoX
changed the title
| pub emit_traces: bool, | ||
| /// Whether to return raw plugin response without ApiResponse wrapper | ||
| #[serde(default)] | ||
| pub raw_response: bool, |
There was a problem hiding this comment.
What effect does this have on openapi? Do we lose type safety in SDK?
There was a problem hiding this comment.
Good question.
I have attached sdk PR in description.
I think change is scoped to plugins so no bigger effect.
| pub raw_response: bool, | ||
| /// Whether to allow GET requests to invoke plugin logic | ||
| #[serde(default)] | ||
| pub allow_get_invocation: bool, |
There was a problem hiding this comment.
So we allow only GET and POST? Do you see a future where someone would want other methods
There was a problem hiding this comment.
Yep, optional GET support was added to be able to implement 3rd party spec.
Hmm, good question.
Potentially yes, maybe other method invocations would be needed in the future. For now it is only GET.
| Ok(HttpResponse::Ok().json(response)) | ||
| if raw_response { | ||
| // Return raw plugin response without ApiResponse wrapper | ||
| Ok(HttpResponse::Ok().json(result)) |
There was a problem hiding this comment.
Is this for a x402 use case?
There was a problem hiding this comment.
Yes, as it is 3rd party and we need to be able to match spec other service requires.
4 participants
Summary
This PR will:
These changes are needed to enable plugin integrations with 3rd party APIs like x402.
SDK PR: OpenZeppelin/openzeppelin-relayer-sdk#240
Testing Process
Checklist
Note
If you are using Relayer in your stack, consider adding your team or organization to our list of Relayer Users in the Wild!