test: add unit tests for config payload and webhook handling#284
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR adds comprehensive unit and integration tests across Rust agent-runtime components and Vue.js web applications to improve code coverage, specifically targeting high-impact uncovered files identified by SonarQube (issue Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Contributor ReportUser: @yacosta738
Contributor Report evaluates based on public GitHub activity. Analysis period: 2025-03-20 to 2026-03-20 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/agent-runtime/src/hardware/introspect.rs`:
- Around line 130-138: The test
memory_map_for_board_uses_static_probe_guidance_without_probe_feature should
only run when the probe feature is disabled; change its conditional to gate it
with both hardware and NOT probe (e.g., replace or augment #[cfg(feature =
"hardware")] with #[cfg(all(feature = "hardware", not(feature = "probe")))] on
the test function) so memory_map_for_board is exercised on the static code path
expected by the assertions.
In `@clients/agent-runtime/src/peripherals/mod.rs`:
- Around line 367-388: Add a test assertion covering the case where a board uses
transport "serial" but path is None: update the tests in
validate_board_config_rejects_invalid_entries to include
validate_board_config(&board("uno", "serial", None)).unwrap_err() asserting the
same "serial transport requires a path" error; this ensures
validate_board_config (the function under test) and the board helper are
exercised for the serial-none path branch that uses unwrap_or("") in the
validation logic.
In `@clients/agent-runtime/src/peripherals/uno_q_bridge.rs`:
- Around line 193-202: Add a test for type-mismatch and make the tool return a
clear type-error instead of a missing-parameter error: update or add a tokio
test (e.g., write_tool_rejects_invalid_pin_type) that calls
UnoQGpioWriteTool.execute with json!({ "pin": "13", "value": 1 }) and asserts
the returned error mentions an invalid pin type; then modify
UnoQGpioWriteTool::execute to validate parameter types (parse/expect pin as
integer and value as integer/bool as appropriate) and return an explicit error
like "Invalid 'pin' type" (instead of "Missing 'pin' parameter") when the JSON
field exists but has the wrong type.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3d44cb32-e685-47f6-ba9c-9879a2895858
📒 Files selected for processing (7)
clients/agent-runtime/src/hardware/introspect.rsclients/agent-runtime/src/peripherals/mod.rsclients/agent-runtime/src/peripherals/uno_q_bridge.rsclients/agent-runtime/src/peripherals/uno_q_setup.rsclients/web/apps/chat/src/App.spec.tsclients/web/apps/dashboard/src/composables/configPayload.spec.tsclients/web/apps/dashboard/src/composables/useConfig.spec.ts
📜 Review details
⏰ 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). (4)
- GitHub Check: sonar
- GitHub Check: pr-checks
- GitHub Check: pr-checks
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (4)
clients/agent-runtime/src/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
clients/agent-runtime/src/**/*.rs: Never log secrets, tokens, raw credentials, or sensitive payloads in any logging statements
Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency
Files:
clients/agent-runtime/src/hardware/introspect.rsclients/agent-runtime/src/peripherals/uno_q_bridge.rsclients/agent-runtime/src/peripherals/uno_q_setup.rsclients/agent-runtime/src/peripherals/mod.rs
clients/agent-runtime/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Run
cargo fmt --all -- --check,cargo clippy --all-targets -- -D warnings, andcargo testfor code validation, or document which checks were skipped and why
Files:
clients/agent-runtime/src/hardware/introspect.rsclients/agent-runtime/src/peripherals/uno_q_bridge.rsclients/agent-runtime/src/peripherals/uno_q_setup.rsclients/agent-runtime/src/peripherals/mod.rs
**/*.rs
⚙️ CodeRabbit configuration file
**/*.rs: Focus on Rust idioms, memory safety, and ownership/borrowing correctness.
Flag unnecessary clones, unchecked panics in production paths, and weak error context.
Prioritize unsafe blocks, FFI boundaries, concurrency races, and secret handling.
Files:
clients/agent-runtime/src/hardware/introspect.rsclients/agent-runtime/src/peripherals/uno_q_bridge.rsclients/agent-runtime/src/peripherals/uno_q_setup.rsclients/agent-runtime/src/peripherals/mod.rs
**/*
⚙️ CodeRabbit configuration file
**/*: Security first, performance second.
Validate input boundaries, auth/authz implications, and secret management.
Look for behavioral regressions, missing tests, and contract breaks across modules.
Files:
clients/agent-runtime/src/hardware/introspect.rsclients/web/apps/chat/src/App.spec.tsclients/agent-runtime/src/peripherals/uno_q_bridge.rsclients/agent-runtime/src/peripherals/uno_q_setup.rsclients/agent-runtime/src/peripherals/mod.rsclients/web/apps/dashboard/src/composables/configPayload.spec.tsclients/web/apps/dashboard/src/composables/useConfig.spec.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/**/*.rs : Run `cargo fmt --all -- --check`, `cargo clippy --all-targets -- -D warnings`, and `cargo test` for code validation, or document which checks were skipped and why
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/**/*.rs : Run `cargo fmt --all -- --check`, `cargo clippy --all-targets -- -D warnings`, and `cargo test` for code validation, or document which checks were skipped and why
Applied to files:
clients/agent-runtime/src/hardware/introspect.rsclients/agent-runtime/src/peripherals/uno_q_bridge.rsclients/agent-runtime/src/peripherals/uno_q_setup.rsclients/agent-runtime/src/peripherals/mod.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/tools/**/*.rs : Implement `Tool` trait in `src/tools/` with strict parameter schema, validate and sanitize all inputs, and return structured `ToolResult` without panics in runtime path
Applied to files:
clients/agent-runtime/src/peripherals/uno_q_bridge.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/channels/**/*.rs : Implement `Channel` trait in `src/channels/` with consistent `send`, `listen`, and `health_check` semantics and cover auth/allowlist/health behavior with tests
Applied to files:
clients/agent-runtime/src/peripherals/mod.rs
🔇 Additional comments (20)
clients/web/apps/chat/src/App.spec.ts (3)
155-170: LGTM - Insecure URL validation test.Good coverage for the
isUrlSafeForSecretsguard. The test correctly verifies that secrets are blocked from being sent to non-HTTPS, non-local URLs.Consider adding a complementary test for allowed local HTTP (e.g.,
http://localhost:3000) to verify the allowlist works as expected.
172-206: LGTM - Webhook secret header test.Solid test verifying that the
X-Webhook-Secretheader is correctly attached to webhook requests after local configuration save.
208-225: LGTM - Timeout error handling test.Correctly simulates an
AbortErrorto verify timeout messaging. The test aligns with howAbortController.abort()behaves in the browser.clients/web/apps/dashboard/src/composables/useConfig.spec.ts (4)
207-233: LGTM - Read-only config fetch test.Good coverage for the unauthenticated public config fetch path. Verifies
observabilityBackendOptionsis populated and status is set correctly.
235-252: LGTM - Pairing without auto-connect test.Cleanly tests the
autoConnect: falsepath, verifying the token is stored without triggering the follow-up connect flow.
254-283: LGTM - Restart-required conflict test.Correctly tests the 409 response handling with field interpolation. The custom
t()mock at lines 273-275 nicely validates parameter passing.
285-306: LGTM - No-changes save optimization test.Good coverage for the empty-payload guard that prevents unnecessary save requests. Validates the UX feedback message as well.
clients/web/apps/dashboard/src/composables/configPayload.spec.ts (6)
6-36: LGTM - Comprehensive snapshot fixture.The
createSnapshot()helper covers all 24 requiredAdminConfigSnapshotfields per the type definition. Good baseline for diff testing.
38-72: LGTM - Form fixture with overrides.Clean pattern deriving form values from the snapshot to maintain consistency. The override spread enables targeted mutations per test.
74-91: LGTM - Diff-only payload with invalid input filtering.Good coverage: verifies changed fields are included while unparseable values (
"not-a-number") are correctly omitted.
93-117: LGTM - Security section nesting test.Validates the nested
autonomyandidentityobject structure, and correctly excludes empty numeric fields.
119-174: LGTM - Multi-section payload construction.Thorough coverage of observability, runtime, scheduler, and gateway sections including the empty-diff case at line 140.
176-220: LGTM - Webhook secret flow tests.Good security coverage: tests
clearmode,replacewith whitespace trimming, and rejection of empty secrets. The whitespace-padded input at line 199 validates the.trim()behavior inbuildWebhookSecretUpdate.clients/agent-runtime/src/peripherals/uno_q_setup.rs (2)
149-164: LGTM — consider verifying file content for stronger guarantees.The test correctly validates the file structure is created. For extra confidence, you could assert that
app.yamlcontains expected content (e.g., a known substring from the embedded file), but this is optional.[approve_code_changes, suggest_optional_refactor]
166-185: LGTM — good recursive copy coverage.Clean test that verifies both the recursive directory handling and content preservation.
clients/agent-runtime/src/peripherals/uno_q_bridge.rs (1)
164-179: LGTM — solid schema and validation tests.Good coverage of the required parameter contract. The tests correctly verify schema structure and parameter validation.
clients/agent-runtime/src/peripherals/mod.rs (2)
330-337: LGTM — clean test helper.The
board()helper reduces boilerplate and improves test readability.
390-409: LGTM — good feature-gated test for bridge tools.Correctly tests both positive (bridge transport adds tools) and negative (serial transport doesn't) cases. The
#[cfg(feature = "hardware")]guard ensures this test only runs when the feature is enabled.clients/agent-runtime/src/hardware/introspect.rs (2)
140-153: Good coverage for value retention inIntrospectResult.This test cleanly validates that supplied fields are preserved (Line 151 and Line 152).
126-154: Please attach Rust validation results for this changed.rsfile.I don’t see evidence in this PR context that
cargo fmt --all -- --check,cargo clippy --all-targets -- -D warnings, andcargo testwere run (or explicitly skipped with rationale).Based on learnings:
Run cargo fmt --all -- --check,cargo clippy --all-targets -- -D warnings, andcargo testfor code validation, or document which checks were skipped and why.
| #[cfg(feature = "hardware")] | ||
| #[test] | ||
| fn memory_map_for_board_uses_static_probe_guidance_without_probe_feature() { | ||
| let note = memory_map_for_board(Some("nucleo-f401re")); | ||
| assert!(note.contains("Build with --features probe")); | ||
|
|
||
| let unknown = memory_map_for_board(Some("custom-board")); | ||
| assert!(unknown.contains("Build with --features probe")); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Read-only check: confirm current cfg gates around this test and probe path.
rg -n -C2 'memory_map_for_board_uses_static_probe_guidance_without_probe_feature|#\[cfg\(feature = "hardware"\)\]|#\[cfg\(feature = "probe"\)\]|fn memory_map_for_board|fn probe_memory_map' clients/agent-runtime/src/hardware/introspect.rsRepository: dallay/corvus
Length of output: 1183
Gate this test to probe-disabled builds only.
Line 130 uses #[cfg(feature = "hardware")], but the inner conditional at line 66 means memory_map_for_board takes a different code path when probe is enabled. Running this test with --all-features will cause it to fail because the function returns probe-based output instead of the static guidance the test expects.
Suggested patch
- #[cfg(feature = "hardware")]
+ #[cfg(all(feature = "hardware", not(feature = "probe")))]
#[test]
fn memory_map_for_board_uses_static_probe_guidance_without_probe_feature() {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/agent-runtime/src/hardware/introspect.rs` around lines 130 - 138, The
test memory_map_for_board_uses_static_probe_guidance_without_probe_feature
should only run when the probe feature is disabled; change its conditional to
gate it with both hardware and NOT probe (e.g., replace or augment #[cfg(feature
= "hardware")] with #[cfg(all(feature = "hardware", not(feature = "probe")))] on
the test function) so memory_map_for_board is exercised on the static code path
expected by the assertions.
| #[test] | ||
| fn validate_board_config_rejects_invalid_entries() { | ||
| assert_eq!( | ||
| validate_board_config(&board("", "serial", Some("/dev/ttyACM0"))).unwrap_err(), | ||
| "board name is empty" | ||
| ); | ||
| assert_eq!( | ||
| validate_board_config(&board("uno", "serial", Some(" "))).unwrap_err(), | ||
| "serial transport requires a path" | ||
| ); | ||
| assert_eq!( | ||
| validate_board_config(&board("uno", "bluetooth", None)).unwrap_err(), | ||
| "unsupported transport 'bluetooth' (use: serial, native, bridge)" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn validate_board_config_accepts_supported_transports() { | ||
| assert!(validate_board_config(&board("uno", "native", None)).is_ok()); | ||
| assert!(validate_board_config(&board("uno", "bridge", None)).is_ok()); | ||
| assert!(validate_board_config(&board("uno", "serial", Some("/dev/ttyUSB0"))).is_ok()); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Good validation coverage — optionally test serial with None path.
The tests cover key rejection scenarios. One minor gap: serial transport with path: None is also rejected by the validation logic (line 217 unwrap_or("")), but isn't explicitly tested. Consider adding for completeness:
Optional additional assertion
assert_eq!(
validate_board_config(&board("uno", "serial", Some(" "))).unwrap_err(),
"serial transport requires a path"
);
+ assert_eq!(
+ validate_board_config(&board("uno", "serial", None)).unwrap_err(),
+ "serial transport requires a path"
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #[test] | |
| fn validate_board_config_rejects_invalid_entries() { | |
| assert_eq!( | |
| validate_board_config(&board("", "serial", Some("/dev/ttyACM0"))).unwrap_err(), | |
| "board name is empty" | |
| ); | |
| assert_eq!( | |
| validate_board_config(&board("uno", "serial", Some(" "))).unwrap_err(), | |
| "serial transport requires a path" | |
| ); | |
| assert_eq!( | |
| validate_board_config(&board("uno", "bluetooth", None)).unwrap_err(), | |
| "unsupported transport 'bluetooth' (use: serial, native, bridge)" | |
| ); | |
| } | |
| #[test] | |
| fn validate_board_config_accepts_supported_transports() { | |
| assert!(validate_board_config(&board("uno", "native", None)).is_ok()); | |
| assert!(validate_board_config(&board("uno", "bridge", None)).is_ok()); | |
| assert!(validate_board_config(&board("uno", "serial", Some("/dev/ttyUSB0"))).is_ok()); | |
| } | |
| #[test] | |
| fn validate_board_config_rejects_invalid_entries() { | |
| assert_eq!( | |
| validate_board_config(&board("", "serial", Some("/dev/ttyACM0"))).unwrap_err(), | |
| "board name is empty" | |
| ); | |
| assert_eq!( | |
| validate_board_config(&board("uno", "serial", Some(" "))).unwrap_err(), | |
| "serial transport requires a path" | |
| ); | |
| assert_eq!( | |
| validate_board_config(&board("uno", "serial", None)).unwrap_err(), | |
| "serial transport requires a path" | |
| ); | |
| assert_eq!( | |
| validate_board_config(&board("uno", "bluetooth", None)).unwrap_err(), | |
| "unsupported transport 'bluetooth' (use: serial, native, bridge)" | |
| ); | |
| } | |
| #[test] | |
| fn validate_board_config_accepts_supported_transports() { | |
| assert!(validate_board_config(&board("uno", "native", None)).is_ok()); | |
| assert!(validate_board_config(&board("uno", "bridge", None)).is_ok()); | |
| assert!(validate_board_config(&board("uno", "serial", Some("/dev/ttyUSB0"))).is_ok()); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/agent-runtime/src/peripherals/mod.rs` around lines 367 - 388, Add a
test assertion covering the case where a board uses transport "serial" but path
is None: update the tests in validate_board_config_rejects_invalid_entries to
include validate_board_config(&board("uno", "serial", None)).unwrap_err()
asserting the same "serial transport requires a path" error; this ensures
validate_board_config (the function under test) and the board helper are
exercised for the serial-none path branch that uses unwrap_or("") in the
validation logic.
| #[tokio::test] | ||
| async fn write_tool_requires_both_parameters() { | ||
| let tool = UnoQGpioWriteTool; | ||
|
|
||
| let missing_pin = tool.execute(json!({ "value": 1 })).await.unwrap_err(); | ||
| assert_eq!(missing_pin.to_string(), "Missing 'pin' parameter"); | ||
|
|
||
| let missing_value = tool.execute(json!({ "pin": 13 })).await.unwrap_err(); | ||
| assert_eq!(missing_value.to_string(), "Missing 'value' parameter"); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider testing type-mismatch scenarios.
Current tests verify missing parameters. You might also test what happens when pin is the wrong type (e.g., json!({ "pin": "13", "value": 1 })). The current implementation would return "Missing 'pin' parameter" which is slightly misleading for a type error. This is optional but would improve diagnostic clarity.
Example additional test
#[tokio::test]
async fn write_tool_rejects_invalid_pin_type() {
let tool = UnoQGpioWriteTool;
let error = tool.execute(json!({ "pin": "13", "value": 1 })).await.unwrap_err();
// Currently returns "Missing 'pin' parameter" — could be improved to "Invalid 'pin' type"
assert!(error.to_string().contains("pin"));
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/agent-runtime/src/peripherals/uno_q_bridge.rs` around lines 193 -
202, Add a test for type-mismatch and make the tool return a clear type-error
instead of a missing-parameter error: update or add a tokio test (e.g.,
write_tool_rejects_invalid_pin_type) that calls UnoQGpioWriteTool.execute with
json!({ "pin": "13", "value": 1 }) and asserts the returned error mentions an
invalid pin type; then modify UnoQGpioWriteTool::execute to validate parameter
types (parse/expect pin as integer and value as integer/bool as appropriate) and
return an explicit error like "Invalid 'pin' type" (instead of "Missing 'pin'
parameter") when the JSON field exists but has the wrong type.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
yacosta738
deleted the
fix/253-improve-sonarqube-coverage-in-the-highest-impact-uncovered-files
branch
1 participant
This pull request adds extensive test coverage across several Rust and TypeScript modules, focusing on hardware introspection, peripheral management, and web client configuration logic. The main goal is to improve reliability and ensure correct behavior for configuration flows, board validation, and hardware tool interfaces.
New and enhanced test coverage:
Hardware and Peripheral Modules:
probe_memory_map,memory_map_for_board, andIntrospectResultto verify correct handling of probe guidance and field retention.peripherals/mod.rs.uno_q_bridge.rs, ensuring correct error handling and schema exposure.uno_q_setup.rs, verifying file structure creation and recursive copying.Web Client Configuration and Payload Logic:
useConfigcomposable, covering read-only config fetches, pairing behavior, restart-required conflicts, and save optimizations.Closes: #253