Skip to content

refactor(agent-runtime): extract tool traits into corvus-traits#506

Merged
yacosta738 merged 3 commits into
feature/capability-architecturefrom
feature/capability-architecture-431-tools-traits
Apr 11, 2026
Merged

refactor(agent-runtime): extract tool traits into corvus-traits#506
yacosta738 merged 3 commits into
feature/capability-architecturefrom
feature/capability-architecture-431-tools-traits

Conversation

@yacosta738
Copy link
Copy Markdown
Contributor

Related Issues

closes #431


Summary

  • extract the Tool contract, metadata types, and default spec() implementation into clients/agent-runtime/crates/corvus-traits/src/tools.rs
  • keep clients/agent-runtime/src/tools/traits.rs as a compatibility shim so legacy imports still resolve to the same tool trait and metadata identities
  • preserve Tool::spec() validation behavior with a minimal extracted schema validator and add compatibility coverage for legacy/new tool paths

Tested Information

  • cargo test --manifest-path Cargo.toml --test traits_api_compat (RED: failed before implementation because corvus_traits::tools did not exist)
  • cargo test --manifest-path Cargo.toml --test traits_api_compat
  • cargo test --manifest-path Cargo.toml -p corvus-traits tools::tests
  • cargo clippy --manifest-path Cargo.toml -p corvus-traits --all-targets -- -D warnings
  • cargo clippy --manifest-path Cargo.toml -p corvus --test traits_api_compat -- -D warnings
  • cargo fmt --all -- --check
  • cargo test (ran via pre-push hook; 3507 passed)

Documentation Impact

  • Docs updated in:
  • No docs update required because this PR only moves internal Rust tool contracts into corvus-traits and preserves the existing public/runtime behavior through a compatibility shim.
  • I verified the documentation matches the current behavior.

Breaking Changes

None.


Checklist

  • I have checked that there isn’t already a PR solving the same problem.
  • I have read the Contributing Guidelines.
  • I ensured my code follows the project's style guidelines.
  • I have added or updated tests that prove my fix is effective or that my feature works.
  • I have updated the documentation, or I explained above why no documentation update is needed.
  • I verified the documentation matches the current behavior.
  • I have documented any breaking changes in the Breaking Changes section.
  • I have linked the related issue (if any).

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 11, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (2)
  • main
  • develop

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6e1353db-3f42-4e54-9b60-68054d65b5c4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/capability-architecture-431-tools-traits

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 11, 2026

✅ Contributor Report

User: @yacosta738
Status: Passed (12/13 metrics passed)

Metric Description Value Threshold Status
PR Merge Rate PRs merged vs closed 89% >= 30%
Repo Quality Repos with ≥100 stars 0 >= 0
Positive Reactions Positive reactions received 10 >= 1
Negative Reactions Negative reactions received 0 <= 5
Account Age GitHub account age 3087 days >= 30 days
Activity Consistency Regular activity over time 108% >= 0%
Issue Engagement Issues with community engagement 0 >= 0
Code Reviews Code reviews given to others 540 >= 0
Merger Diversity Unique maintainers who merged PRs 2 >= 0
Repo History Merge Rate Merge rate in this repo 92% >= 0%
Repo History Min PRs Previous PRs in this repo 226 >= 0
Profile Completeness Profile richness (bio, followers) 90 >= 0
Suspicious Patterns Spam-like activity detection 1 N/A

Contributor Report evaluates based on public GitHub activity. Analysis period: 2025-04-11 to 2026-04-11

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Apr 11, 2026

Deploying corvus with  Cloudflare Pages  Cloudflare Pages

Latest commit: b3a264b
Status:⚡️  Build in progress...

View logs

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/crates/corvus-traits/src/testing.rs`:
- Around line 4-6: The testing harness struct is currently declared pub(crate)
(TraitHarness) while the crate still exposes pub mod testing, so downstream
crates cannot use the harness; either make the harness fully exported by
changing TraitHarness's visibility from pub(crate) to pub, or if the harness
should remain internal, stop exporting the module by removing or changing the
pub mod testing declaration in lib.rs; locate the TraitHarness declaration in
testing.rs and adjust its visibility to pub, or update lib.rs to make the
testing module private to resolve the mismatch.

In `@clients/agent-runtime/crates/corvus-traits/src/tools.rs`:
- Around line 92-105: validate_tool_schema currently accepts any schema with a
"type" field, allowing non-object types (e.g., "type":"string") which breaks
function-calling parameter binding; update validate_tool_schema so that if a
"type" field exists it must be the string "object" (otherwise return an error
via anyhow::bail), and keep the existing warning when an object schema lacks
"properties"; reference validate_tool_schema and the "type"/"properties" keys
when making the change so spec() only receives object-typed parameter schemas.

In `@clients/agent-runtime/tests/traits_api_compat.rs`:
- Around line 167-182: Extend the TypeId compatibility assertions to cover the
moved metadata types: add assert_eq! checks comparing
TypeId::of::<ToolSourceMetadata>() and each descriptor hint struct re-exported
by the shim (the same names as in corvus_traits, e.g., any ToolDescriptorHint
types) against their corvus_traits counterparts
(TypeId::of::<corvus_traits::...>()) just like the existing assertions for
RuntimeTool, ToolResult, and ToolSpec; place these new assert_eq! lines
alongside the existing assertions so the test fails if those re-exports drift.
🪄 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: 4680d0fd-a52b-4741-983c-c7315ac263aa

📥 Commits

Reviewing files that changed from the base of the PR and between cef3193 and 5527713.

⛔ Files ignored due to path filters (1)
  • clients/agent-runtime/Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (7)
  • clients/agent-runtime/crates/corvus-traits/Cargo.toml
  • clients/agent-runtime/crates/corvus-traits/src/channels.rs
  • clients/agent-runtime/crates/corvus-traits/src/lib.rs
  • clients/agent-runtime/crates/corvus-traits/src/testing.rs
  • clients/agent-runtime/crates/corvus-traits/src/tools.rs
  • clients/agent-runtime/src/tools/traits.rs
  • clients/agent-runtime/tests/traits_api_compat.rs
📜 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). (1)
  • GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (8)
clients/agent-runtime/**/*.rs

📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)

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

Files:

  • clients/agent-runtime/crates/corvus-traits/src/lib.rs
  • clients/agent-runtime/crates/corvus-traits/src/testing.rs
  • clients/agent-runtime/crates/corvus-traits/src/channels.rs
  • clients/agent-runtime/tests/traits_api_compat.rs
  • clients/agent-runtime/src/tools/traits.rs
  • clients/agent-runtime/crates/corvus-traits/src/tools.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/crates/corvus-traits/src/lib.rs
  • clients/agent-runtime/crates/corvus-traits/src/testing.rs
  • clients/agent-runtime/crates/corvus-traits/src/channels.rs
  • clients/agent-runtime/tests/traits_api_compat.rs
  • clients/agent-runtime/src/tools/traits.rs
  • clients/agent-runtime/crates/corvus-traits/src/tools.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/crates/corvus-traits/src/lib.rs
  • clients/agent-runtime/crates/corvus-traits/src/testing.rs
  • clients/agent-runtime/crates/corvus-traits/Cargo.toml
  • clients/agent-runtime/crates/corvus-traits/src/channels.rs
  • clients/agent-runtime/tests/traits_api_compat.rs
  • clients/agent-runtime/src/tools/traits.rs
  • clients/agent-runtime/crates/corvus-traits/src/tools.rs
clients/agent-runtime/**/Cargo.toml

📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)

clients/agent-runtime/**/Cargo.toml: Preserve release-size profile assumptions in Cargo.toml and avoid adding heavy dependencies unless clearly justified
Do not add heavy dependencies for minor convenience; justify new crate additions

Files:

  • clients/agent-runtime/crates/corvus-traits/Cargo.toml
clients/agent-runtime/src/tools/**/*.rs

📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)

Implement Tool trait in src/tools/ with strict parameter schema, validate and sanitize all inputs, and return structured ToolResult without panics in runtime path

Files:

  • clients/agent-runtime/src/tools/traits.rs
clients/agent-runtime/src/{security,gateway,tools}/**/*.rs

📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)

Treat src/security/, src/gateway/, src/tools/ as high-risk surfaces and never broaden filesystem/network execution scope without explicit policy checks

Files:

  • clients/agent-runtime/src/tools/traits.rs
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/tools/traits.rs
clients/agent-runtime/src/{security,gateway,tools,config}/**/*.rs

📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)

Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable

Files:

  • clients/agent-runtime/src/tools/traits.rs
🧠 Learnings (11)
📓 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/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
📚 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/crates/corvus-traits/src/lib.rs
  • clients/agent-runtime/tests/traits_api_compat.rs
  • clients/agent-runtime/src/tools/traits.rs
  • clients/agent-runtime/crates/corvus-traits/src/tools.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/**/Cargo.toml : Do not add heavy dependencies for minor convenience; justify new crate additions

Applied to files:

  • clients/agent-runtime/crates/corvus-traits/src/lib.rs
  • clients/agent-runtime/crates/corvus-traits/src/testing.rs
  • clients/agent-runtime/crates/corvus-traits/Cargo.toml
  • clients/agent-runtime/src/tools/traits.rs
  • clients/agent-runtime/crates/corvus-traits/src/tools.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/providers/**/*.rs : Implement `Provider` trait in `src/providers/` and register in `src/providers/mod.rs` factory when adding a new provider

Applied to files:

  • clients/agent-runtime/crates/corvus-traits/src/lib.rs
  • clients/agent-runtime/src/tools/traits.rs
  • clients/agent-runtime/crates/corvus-traits/src/tools.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/{security,gateway,tools}/**/*.rs : Treat `src/security/`, `src/gateway/`, `src/tools/` as high-risk surfaces and never broaden filesystem/network execution scope without explicit policy checks

Applied to files:

  • clients/agent-runtime/crates/corvus-traits/src/lib.rs
  • clients/agent-runtime/src/tools/traits.rs
  • clients/agent-runtime/crates/corvus-traits/src/tools.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/main.rs : Preserve CLI contract unless change is intentional and documented; prefer explicit errors over silent fallback for unsupported critical paths

Applied to files:

  • clients/agent-runtime/crates/corvus-traits/src/lib.rs
  • clients/agent-runtime/crates/corvus-traits/Cargo.toml
  • clients/agent-runtime/tests/traits_api_compat.rs
  • clients/agent-runtime/src/tools/traits.rs
  • clients/agent-runtime/crates/corvus-traits/src/tools.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/**/*.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/crates/corvus-traits/src/lib.rs
  • clients/agent-runtime/crates/corvus-traits/src/testing.rs
  • clients/agent-runtime/crates/corvus-traits/Cargo.toml
  • clients/agent-runtime/tests/traits_api_compat.rs
  • clients/agent-runtime/src/tools/traits.rs
  • clients/agent-runtime/crates/corvus-traits/src/tools.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/**/Cargo.toml : Preserve release-size profile assumptions in `Cargo.toml` and avoid adding heavy dependencies unless clearly justified

Applied to files:

  • clients/agent-runtime/crates/corvus-traits/src/lib.rs
  • clients/agent-runtime/crates/corvus-traits/src/testing.rs
  • clients/agent-runtime/crates/corvus-traits/Cargo.toml
  • clients/agent-runtime/src/tools/traits.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/crates/corvus-traits/src/channels.rs
  • clients/agent-runtime/tests/traits_api_compat.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/{security,gateway,tools,config}/**/*.rs : Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable

Applied to files:

  • clients/agent-runtime/src/tools/traits.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/**/*.rs : Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency

Applied to files:

  • clients/agent-runtime/src/tools/traits.rs
🔇 Additional comments (3)
clients/agent-runtime/crates/corvus-traits/Cargo.toml (1)

15-19: Nice tokio feature split.

Keeping sync in normal deps and moving macros/rt into dev-dependencies preserves the smaller runtime surface while still supporting the crate’s async tests.

Based on learnings "Preserve release-size profile assumptions in Cargo.toml and avoid adding heavy dependencies unless clearly justified".

clients/agent-runtime/crates/corvus-traits/src/channels.rs (1)

70-74: Good compatibility fallback.

Returning legacy content when the multimodal parts contain no extractable text prevents empty projections for older text-only callers.

clients/agent-runtime/src/tools/traits.rs (1)

3-6: Good compatibility shim.

Directly re-exporting the canonical symbols is the right shape here; it preserves trait and type identity for legacy imports instead of introducing adapter drift.

Comment on lines +4 to +6
#[allow(dead_code)]
#[derive(Debug, Default, Clone, Copy)]
pub struct TraitHarness;
pub(crate) struct TraitHarness;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Keep the exported harness public, or make the module private.

clients/agent-runtime/crates/corvus-traits/src/lib.rs still exposes pub mod testing;, but TraitHarness is now pub(crate). That leaves downstream crates with no usable compliance-test surface, which undercuts the extraction goal for shared harness scaffolding.

Suggested fix
-#[allow(dead_code)]
 #[derive(Debug, Default, Clone, Copy)]
-pub(crate) struct TraitHarness;
+pub struct TraitHarness;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@clients/agent-runtime/crates/corvus-traits/src/testing.rs` around lines 4 -
6, The testing harness struct is currently declared pub(crate) (TraitHarness)
while the crate still exposes pub mod testing, so downstream crates cannot use
the harness; either make the harness fully exported by changing TraitHarness's
visibility from pub(crate) to pub, or if the harness should remain internal,
stop exporting the module by removing or changing the pub mod testing
declaration in lib.rs; locate the TraitHarness declaration in testing.rs and
adjust its visibility to pub, or update lib.rs to make the testing module
private to resolve the mismatch.

Comment on lines +92 to +105
fn validate_tool_schema(schema: &serde_json::Value) -> anyhow::Result<()> {
let obj = schema
.as_object()
.ok_or_else(|| anyhow::anyhow!("Schema must be an object"))?;

if !obj.contains_key("type") {
anyhow::bail!("Schema missing required 'type' field");
}

if let Some(serde_json::Value::String(schema_type)) = obj.get("type") {
if schema_type == "object" && !obj.contains_key("properties") {
tracing::warn!("Object schema without 'properties' field may cause issues");
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Reject non-object parameter schemas.

validate_tool_schema() currently accepts any schema that merely has a "type" field, so "type": "string" passes spec(). For tool/function-calling parameters, that produces a spec the caller cannot bind like an argument object.

Suggested tightening
 fn validate_tool_schema(schema: &serde_json::Value) -> anyhow::Result<()> {
     let obj = schema
         .as_object()
         .ok_or_else(|| anyhow::anyhow!("Schema must be an object"))?;
 
-    if !obj.contains_key("type") {
-        anyhow::bail!("Schema missing required 'type' field");
-    }
-
-    if let Some(serde_json::Value::String(schema_type)) = obj.get("type") {
-        if schema_type == "object" && !obj.contains_key("properties") {
-            tracing::warn!("Object schema without 'properties' field may cause issues");
-        }
-    }
+    match obj.get("type") {
+        Some(serde_json::Value::String(schema_type)) if schema_type == "object" => {
+            if !obj.contains_key("properties") && !obj.contains_key("additionalProperties") {
+                tracing::warn!(
+                    "Object schema without 'properties' or 'additionalProperties' may cause issues"
+                );
+            }
+        }
+        Some(serde_json::Value::String(schema_type)) => {
+            anyhow::bail!("Tool parameter schema must have type 'object', got '{schema_type}'");
+        }
+        _ => anyhow::bail!("Schema missing required 'type' field"),
+    }
 
     Ok(())
 }
📝 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.

Suggested change
fn validate_tool_schema(schema: &serde_json::Value) -> anyhow::Result<()> {
let obj = schema
.as_object()
.ok_or_else(|| anyhow::anyhow!("Schema must be an object"))?;
if !obj.contains_key("type") {
anyhow::bail!("Schema missing required 'type' field");
}
if let Some(serde_json::Value::String(schema_type)) = obj.get("type") {
if schema_type == "object" && !obj.contains_key("properties") {
tracing::warn!("Object schema without 'properties' field may cause issues");
}
}
fn validate_tool_schema(schema: &serde_json::Value) -> anyhow::Result<()> {
let obj = schema
.as_object()
.ok_or_else(|| anyhow::anyhow!("Schema must be an object"))?;
match obj.get("type") {
Some(serde_json::Value::String(schema_type)) if schema_type == "object" => {
if !obj.contains_key("properties") && !obj.contains_key("additionalProperties") {
tracing::warn!(
"Object schema without 'properties' or 'additionalProperties' may cause issues"
);
}
}
Some(serde_json::Value::String(schema_type)) => {
anyhow::bail!("Tool parameter schema must have type 'object', got '{schema_type}'");
}
_ => anyhow::bail!("Schema missing required 'type' field"),
}
Ok(())
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@clients/agent-runtime/crates/corvus-traits/src/tools.rs` around lines 92 -
105, validate_tool_schema currently accepts any schema with a "type" field,
allowing non-object types (e.g., "type":"string") which breaks function-calling
parameter binding; update validate_tool_schema so that if a "type" field exists
it must be the string "object" (otherwise return an error via anyhow::bail), and
keep the existing warning when an object schema lacks "properties"; reference
validate_tool_schema and the "type"/"properties" keys when making the change so
spec() only receives object-typed parameter schemas.

Comment on lines +167 to +182
assert_eq!(
TypeId::of::<&dyn RuntimeTool>(),
TypeId::of::<&dyn corvus_traits::tools::Tool>()
);
assert_eq!(
TypeId::of::<&dyn tools::traits::Tool>(),
TypeId::of::<&dyn corvus_traits::tools::Tool>()
);
assert_eq!(
TypeId::of::<RuntimeToolResult>(),
TypeId::of::<corvus_traits::tools::ToolResult>()
);
assert_eq!(
TypeId::of::<RuntimeToolSpec>(),
TypeId::of::<corvus_traits::tools::ToolSpec>()
);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Extend the compat assertions to the moved metadata types.

This suite locks Tool, ToolResult, and ToolSpec, but the shim also re-exports ToolSourceMetadata and the descriptor hint structs. Without asserting those too, part of the extracted public surface can drift unnoticed.

Suggested assertions
     assert_eq!(
         TypeId::of::<RuntimeToolSpec>(),
         TypeId::of::<corvus_traits::tools::ToolSpec>()
     );
+    assert_eq!(
+        TypeId::of::<tools::traits::ToolSourceMetadata>(),
+        TypeId::of::<corvus_traits::tools::ToolSourceMetadata>()
+    );
+    assert_eq!(
+        TypeId::of::<tools::traits::ToolDescriptorHint>(),
+        TypeId::of::<corvus_traits::tools::ToolDescriptorHint>()
+    );
+    assert_eq!(
+        TypeId::of::<tools::traits::ToolDescriptorMcpHint>(),
+        TypeId::of::<corvus_traits::tools::ToolDescriptorMcpHint>()
+    );
+    assert_eq!(
+        TypeId::of::<tools::traits::ToolDescriptorMcpPromptArgumentHint>(),
+        TypeId::of::<corvus_traits::tools::ToolDescriptorMcpPromptArgumentHint>()
+    );
📝 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.

Suggested change
assert_eq!(
TypeId::of::<&dyn RuntimeTool>(),
TypeId::of::<&dyn corvus_traits::tools::Tool>()
);
assert_eq!(
TypeId::of::<&dyn tools::traits::Tool>(),
TypeId::of::<&dyn corvus_traits::tools::Tool>()
);
assert_eq!(
TypeId::of::<RuntimeToolResult>(),
TypeId::of::<corvus_traits::tools::ToolResult>()
);
assert_eq!(
TypeId::of::<RuntimeToolSpec>(),
TypeId::of::<corvus_traits::tools::ToolSpec>()
);
assert_eq!(
TypeId::of::<&dyn RuntimeTool>(),
TypeId::of::<&dyn corvus_traits::tools::Tool>()
);
assert_eq!(
TypeId::of::<&dyn tools::traits::Tool>(),
TypeId::of::<&dyn corvus_traits::tools::Tool>()
);
assert_eq!(
TypeId::of::<RuntimeToolResult>(),
TypeId::of::<corvus_traits::tools::ToolResult>()
);
assert_eq!(
TypeId::of::<RuntimeToolSpec>(),
TypeId::of::<corvus_traits::tools::ToolSpec>()
);
assert_eq!(
TypeId::of::<tools::traits::ToolSourceMetadata>(),
TypeId::of::<corvus_traits::tools::ToolSourceMetadata>()
);
assert_eq!(
TypeId::of::<tools::traits::ToolDescriptorHint>(),
TypeId::of::<corvus_traits::tools::ToolDescriptorHint>()
);
assert_eq!(
TypeId::of::<tools::traits::ToolDescriptorMcpHint>(),
TypeId::of::<corvus_traits::tools::ToolDescriptorMcpHint>()
);
assert_eq!(
TypeId::of::<tools::traits::ToolDescriptorMcpPromptArgumentHint>(),
TypeId::of::<corvus_traits::tools::ToolDescriptorMcpPromptArgumentHint>()
);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@clients/agent-runtime/tests/traits_api_compat.rs` around lines 167 - 182,
Extend the TypeId compatibility assertions to cover the moved metadata types:
add assert_eq! checks comparing TypeId::of::<ToolSourceMetadata>() and each
descriptor hint struct re-exported by the shim (the same names as in
corvus_traits, e.g., any ToolDescriptorHint types) against their corvus_traits
counterparts (TypeId::of::<corvus_traits::...>()) just like the existing
assertions for RuntimeTool, ToolResult, and ToolSpec; place these new assert_eq!
lines alongside the existing assertions so the test fails if those re-exports
drift.

@yacosta738 yacosta738 merged commit 6df8efa into feature/capability-architecture Apr 11, 2026
6 of 7 checks passed
@yacosta738 yacosta738 deleted the feature/capability-architecture-431-tools-traits branch April 11, 2026 07:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant