feat(tools): expose ~160 agent tools across 23 core domains (overextending tools default-OFF)#3050
Merged
senamakel merged 12 commits intoMay 31, 2026
Merged
Conversation
vault: list, get, files, create, sync, sync_status, ★remove people: list, resolve, score, get, add_alias, record_interaction, ★refresh_address_book Thin Tool-trait shims over vault::ops (RpcOutcome) and people::rpc/store. Read/bounded-write default-ON; vault_remove (irreversible unregister+purge) and people_refresh_address_book (bulk OS contacts ingest + permission prompt) ship default-OFF via tools/user_filter.rs. vault_write_markdown already existed and is not duplicated. Co-located unit tests; full registry + e2e coverage lands with the rest of the knowledge theme.
skills: list, describe, read_resource, recent_runs, read_run_log,
★create, ★install_from_url, ★uninstall
Thin shims over skills::ops_discover/ops_create/ops_install/registry/run_log.
skill_run already exists (RunSkillTool) and is not duplicated. Reads default-ON;
create/install_from_url/uninstall ship default-OFF via the skill_manage toggle
(install_from_url also flags external_effect — it fetches remote content).
Co-located unit tests.
threads: list, read, create, update_title, update_labels, message_list,
message_append, message_update, title_generate, turn_state_get/list/clear,
task_board_read/write, ★delete, ★purge_all
Tools deserialize into the same request structs the RPC layer uses and delegate
to threads::ops (RpcOutcome<ApiEnvelope>); task_board read/write go through
agent::task_board. thread_read filters the list (no dedicated backing fn).
title_generate is Execute (inference). thread_delete/thread_purge_all ship
default-OFF via the thread_destructive toggle.
ops_tests.rs gains knowledge-theme e2e: registration of all 38 knowledge tools,
default-OFF filter posture (filtered when not opted in, retained when opted in),
and a vault_list execution through the boxed registry. Learning domain to follow.
…gent-tools-knowledge
Add 24 agent tools across four RPC-facing domains that previously had no agent-callable surface — agent_workflows, todos, artifacts, task_sources — as thin Tool-trait shims over each domain's existing ops/RPC: - agent_workflows: list, read, phase_info, create, ★uninstall - todos: list, add, edit, update_status, decide_plan, ★remove, ★replace, ★clear - artifacts: list, get, ★delete - task_sources: list, get, fetch, list_tasks, preview_filter, status, ★add, ★update, ★remove Read-only and bounded/reversible writers ship default-ON; the ★ overextending siblings (irreversible deletes + persistent-config mutators) ship default-OFF via tools/user_filter.rs (grouped per-family toggle IDs), matching the established update_* opt-in precedent. Per-call PermissionLevel (ReadOnly/Write/Execute/Dangerous) gates each tool regardless of toggle. Tools registered in tools::ops::all_tools_with_runtime and re-exported via tools/mod.rs. Each tool carries co-located unit tests; tools/ops_tests.rs adds end-to-end coverage: full-registry registration, the default-OFF user-filter posture (filtered when not opted in, retained when opted in), and a real add→list execution through the boxed dyn Tool surface against the on-disk task board. First slice of the agent-tool expansion (Task & workflow productivity); Knowledge/Comms/System/Account themes to follow as separate PRs.
…e theme)
learning: list_facets, get_facet, cache_stats,
★update_facet, ★pin_facet, ★unpin_facet, ★forget_facet,
★rebuild_cache, ★reset_cache, ★save_profile, ★enrich_profile
Mirrors the private learning::schemas handler logic over FacetCache
(memory::global), with identical class/key composition and FacetState/UserState
transitions. Reads default-ON; all 8 mutators default-OFF via learning_manage
toggle (enrich_profile also flags external_effect). Knowledge-theme e2e extended
to cover all 11. Knowledge theme now complete (48 tools).
doctor: health, models
health: snapshot, system_info
cost: get_dashboard, get_daily_history, get_summary
dashboard: model_health
security: policy_info
service: status, daemon_host_prefs_get, ★start, ★stop, ★restart, ★shutdown,
★install, ★uninstall, ★daemon_host_prefs_set
Read/observe tools default-ON; service lifecycle mutators default-OFF via the
service_lifecycle toggle (shutdown/install/uninstall are Dangerous). Only
backing fns that actually exist are wrapped (doctor filter, dashboard
get_settings, and the security command/path checks have no RPC and are skipped).
e2e: registration + default-OFF posture + health_system_info execution.
config: snapshot, get_client_config, get_autonomy, get_search,
get_runtime_flags, resolve_api_url, get_data_paths
Read-only, default-ON. The config_update_* mutators are deferred: their apply
fns take hand-built non-Deserialize *SettingsPatch structs mapped field-by-field
from separate wire types in config::schemas; exposing them needs those wire
types made public (or a config::ops patch-from-json helper). Documented in
config/tools.rs. System-theme e2e extended to cover the config reads.
…, credentials)
billing: 7 reads + ★8 money-movers/payment-method mutators (billing_writes;
delete_card Dangerous; all flag external_effect)
team: 5 reads + ★10 membership/org mutators (team_admin; delete/remove Dangerous)
referral: get_stats, claim
credentials: credential_list, session_state, session_get_user, oauth_connect_url,
oauth_list (READ-ONLY non-secret surface; secret/auth-mutation ops
intentionally NOT exposed)
Reads default-ON; money/admin mutators default-OFF via billing_writes / team_admin
toggles. e2e: registration + default-OFF posture for all 37.
…l theme) screen_intelligence: status, capture_image_ref, vision_recent/flush, refresh_permissions, capture_now/test, session_start/stop, input_action, globe_listener_start/poll/stop, ★request_permissions, ★request_permission mcp_registry: search, get, installed_list, status, connect, disconnect, tool_call, config_assist, ★install, ★uninstall (mcp_registry_* names — distinct from the existing mcp_setup_*/mcp_call_tool tools) workspace: read_persona, ★update_persona, ★reset_persona, ★init Observe/connect/call tools default-ON; OS permission prompts (screen_permissions), MCP install/uninstall (mcp_manage), and persona/workspace writers (workspace_manage) default-OFF. e2e: registration + default-OFF posture for all 28.
Contributor
|
Warning Review limit reached
More reviews will be available in 18 minutes and 18 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR adds ~100 LLM-callable Tool trait implementations across 20+ OpenHuman domains, systematically converting domain operations into agent-accessible tools. Each domain gains a dedicated ChangesTool Surface Expansion
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
|
coderabbitai
Bot
added
feature
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
src/openhuman/tools/ops.rs (1)
83-753: ⚡ Quick winConsider refactoring the lengthy registry function into theme-based helpers.
The
all_tools_with_runtimefunction is now 670+ lines. While the guideline "Prefer modules ≤ ~500 lines" technically applies to modules rather than functions, extracting the five theme-based registration blocks (lines 256-457) into helper functions likeregister_knowledge_tools,register_productivity_tools,register_system_tools,register_account_tools, andregister_desktop_toolswould improve readability and make future additions easier to locate.♻️ Example refactor pattern
fn register_knowledge_tools(config: Arc<Config>) -> Vec<Box<dyn Tool>> { vec![ Box::new(VaultListTool::new(config.clone())), Box::new(VaultGetTool::new(config.clone())), // ... remaining knowledge tools ] } // Then in all_tools_with_runtime: tools.extend(register_knowledge_tools(config.clone())); tools.extend(register_productivity_tools(config.clone())); // ... etcAs per coding guidelines: "Prefer modules ≤ ~500 lines; split growing modules into separate files"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/tools/ops.rs` around lines 83 - 753, The all_tools_with_runtime function is too large; extract the five theme-based registration blocks into small helper builders (e.g. register_knowledge_tools, register_productivity_tools, register_system_tools, register_account_tools, register_desktop_tools) that return Vec<Box<dyn Tool>> and then call tools.extend(...) from all_tools_with_runtime; each helper should accept only the specific parameters it needs (e.g. config: Arc<Config>, security: Arc<SecurityPolicy>, memory: Arc<dyn Memory>, root_config clone, workspace_dir, runtime/audit as needed) and construct the same Box::new(...) entries you moved (e.g. VaultListTool::new, SkillListTool::new, ThreadListTool, LearningListFacetsTool, BillingPlanTool::new, ScreenStatusTool, etc.), then replace the original inline blocks in all_tools_with_runtime with tools.extend(helper(...)) to preserve behavior and keep node/browser/integration gating unchanged.src/openhuman/team/tools.rs (1)
41-70: ⚡ Quick winAdd debug logging to macro-generated tools for observability.
The
cfg_read!macro (and similarlyteam_id_read!andteam_id_mutator!) generates tool implementations without any debug logging. As per coding guidelines, new Rust flows should include verbose diagnostics vialog/tracingat debug/trace levels.📊 Add logging to macros
async fn execute(&self, _args: serde_json::Value) -> anyhow::Result<ToolResult> { + log::debug!(concat!("[tool][team] ", $name, " invoked")); emit!(team::$fn(&self.config).await, $name) }Apply the same pattern to
team_id_read!(line 94) andteam_id_mutator!(line 161).As per coding guidelines, default to verbose diagnostics on new/changed flows using
log/tracingatdebug/tracelevels with stable grep-friendly prefixes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/team/tools.rs` around lines 41 - 70, The macro cfg_read! currently generates Tool impls with no diagnostics; add stable, grep-friendly debug/trace logging (using tracing or log) around construction and execution: instrument the $ty::new (or inside impl Tool::execute) to emit a trace/debug before and after calling emit!(team::$fn(&self.config).await, $name) and log the arguments/tool name and result/error, and apply the same pattern to the other macros team_id_read! and team_id_mutator! so each generated Tool logs entry, exit, and errors with a consistent prefix that includes $name and the invoked function (team::$fn).src/openhuman/learning/tools.rs (1)
41-43: 💤 Low valueConsider propagating or logging serialization errors instead of silently converting to Null.
The
unwrap_or(serde_json::Value::Null)pattern hides any serialization failures forProfileFacet. WhileProfileFacetis an internal type that should serialize reliably, if the schema ever changes unexpectedly or contains unserializable data, this will silently produceNullinstead of surfacing the issue.🔍 Alternative: propagate the error
-fn facet_to_json(f: &ProfileFacet) -> serde_json::Value { - serde_json::to_value(f).unwrap_or(serde_json::Value::Null) +fn facet_to_json(f: &ProfileFacet) -> Result<serde_json::Value, serde_json::Error> { + serde_json::to_value(f) }Then handle the error at call sites, or log a warning before defaulting to Null.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/learning/tools.rs` around lines 41 - 43, The helper facet_to_json currently swallows serde_json::to_value errors by returning Value::Null, which hides serialization failures for ProfileFacet; update facet_to_json to return a Result<serde_json::Value, serde_json::Error> (or Option with logging) so callers can handle errors, or at minimum log the serde_json error before defaulting to Null; locate the function facet_to_json and change its signature and implementation to propagate the serde_json::Error (or call serde_json::to_value(f).map_err(|e| { log::warn!(...); e })) and update all call sites that use facet_to_json to handle the Result/Option accordingly.src/openhuman/cost/tools.rs (1)
87-93: 💤 Low valuePotential truncation when casting u64 to u32 for days parameter.
The JSON schema allows any integer ≥ 1 for
days, which could be a u64, but the code casts it to u32 without bounds checking. If a client passes a value >u32::MAX, it will silently truncate.🛡️ Add bounds validation or constrain the schema
Option 1: Validate the range before casting:
let days = args .get("days") .and_then(serde_json::Value::as_u64) + .filter(|&v| v <= u32::MAX as u64) .map(|v| v as u32) .unwrap_or(30);Option 2: Constrain the schema to a reasonable maximum (e.g., 365 days):
"days": { "type": "integer", "minimum": 1, + "maximum": 365, "description": "How many days back (default 30)." }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/cost/tools.rs` around lines 87 - 93, The days extraction in async fn execute uses .as_u64().map(|v| v as u32) which can silently truncate values > u32::MAX; update the extraction to validate bounds before casting (e.g., check the u64 against u32::MAX and against a sensible application maximum like 365 or return an error), then safely convert (using u32::try_from or an explicit check) and use the validated/capped value for days.src/openhuman/billing/tools.rs (1)
222-225: 🏗️ Heavy liftFloating-point (f64) for monetary amounts can introduce precision errors.
Using
f64to represent money is a known anti-pattern in financial systems due to floating-point rounding and precision issues. This code extractsamount_usdasf64and passes it directly to the billing layer.While fixing this in the tool wrapper alone won't help (the underlying
billing::top_up_creditsAPI already acceptsf64), the broader codebase should consider migrating to integer cents (e.g.,u64of cents) or a decimal type (e.g.,rust_decimal::Decimal) for all monetary values to avoid precision loss.This is an architectural concern beyond the scope of this tool wrapper, but worth flagging for future refactoring.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/billing/tools.rs` around lines 222 - 225, Replace the current extraction of amount_usd as an f64 with a Decimal-based parse: read args.get("amount_usd") into a rust_decimal::Decimal (accepting numeric or string JSON values), multiply by 100 and round to derive integer cents (u64) for internal representation (use a local name like amount_cents or amount_usd_decimal), and then when calling billing::top_up_credits convert explicitly from the Decimal or cents to the API type if needed; update uses of the old local symbol amount to the new symbol (e.g., amount_usd_decimal or amount_cents) and keep the conversion to f64 only at the final API boundary to minimize precision loss (refer to args.get("amount_usd") extraction and the billing::top_up_credits call).src/openhuman/vault/tools.rs (1)
27-52: ⚡ Quick winConsider extracting common tool helpers to a shared module.
The
read_required_str,opt_str_vec, andemit!macro pattern appears across multiple tool files (vault, people, skills, service). Consolidating these intocrate::openhuman::tools::helpersor similar would reduce duplication and ensure consistent validation/error handling across all domains.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/vault/tools.rs` around lines 27 - 52, The helpers read_required_str, opt_str_vec and the emit! macro are duplicated across multiple tool modules; extract them into a shared module (e.g., crate::openhuman::tools::helpers) and replace local definitions with imports. Move the implementations of read_required_str, opt_str_vec and emit! into the new helpers module, export them (pub) and update callers in vault/tools.rs (and people, skills, service tool files) to use crate::openhuman::tools::helpers::{read_required_str, opt_str_vec, emit}. Ensure the emit! macro signature and error-wrapping behavior remain identical so existing call sites (which expect ToolResult::success(serde_json::to_string(&outcome.value)?)) continue to work.src/openhuman/todos/tools.rs (1)
1-602: ⚡ Quick winConsider splitting this module to align with the ~500 line guideline.
The file contains 602 lines with 8 tool implementations. As per coding guidelines, modules should prefer staying under ~500 lines. Consider organizing tools into submodules, for example:
tools/read.rsfor read-only tools (list)tools/write.rsfor bounded writers (add/edit/update_status/decide_plan)tools/destructive.rsfor default-OFF operations (remove/replace/clear)tools/mod.rsfor re-exports and shared helpers (board_location, card_patch, etc.)This would improve maintainability and navigation.
As per coding guidelines: "Prefer modules ≤ ~500 lines; split growing modules into separate files"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/todos/tools.rs` around lines 1 - 602, The module is over the ~500-line guideline; split src/openhuman/todos/tools.rs by moving tool implementations into submodules (e.g., move TodoListTool into tools/read.rs; TodoAddTool, TodoEditTool, TodoUpdateStatusTool, TodoDecidePlanTool into tools/write.rs; TodoRemoveTool, TodoReplaceTool, TodoClearTool into tools/destructive.rs) and keep shared helpers (board_location, card_patch, read_required_str, opt_str, opt_str_vec, snapshot_to_result, thread_id_prop) plus common imports in tools/mod.rs; re-export the tools from mod.rs (pub use crate::openhuman::todos::tools::{TodoListTool, TodoAddTool, ...}) so external callers keep the same names, and update tests to reference the new module layout or move test blocks into the appropriate submodule files.src/openhuman/task_sources/tools.rs (1)
1-571: ⚡ Quick winConsider splitting this module to align with the ~500 line guideline.
The file contains 571 lines with 9 tool implementations. As per coding guidelines, modules should prefer staying under ~500 lines. Consider organizing tools into submodules, for example:
tools/read.rsfor read-only tools (list/get/fetch/list_tasks/preview_filter/status)tools/write.rsfor mutators (add/update/remove)tools/mod.rsfor re-exports and shared helpersThis would improve maintainability and navigation.
As per coding guidelines: "Prefer modules ≤ ~500 lines; split growing modules into separate files"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/task_sources/tools.rs` around lines 1 - 571, Split the large module by moving the read-only tools (TaskSourceListTool, TaskSourceGetTool, TaskSourceFetchTool, TaskSourceListTasksTool, TaskSourcePreviewFilterTool, TaskSourceStatusTool) into a new submodule (e.g., tools::read) and the mutator tools (TaskSourceAddTool, TaskSourceUpdateTool, TaskSourceRemoveTool) into a new submodule (e.g., tools::write); keep the shared helpers and macro (read_required_str, opt_u64, opt_str, parse_provider, parse_filter, emit!) in the current module (or a small shared submodule) with pub(crate) visibility so submodules can use them, then re-export the tool types and constructors from the parent module so external callers keep the same names, and update the tests to reference the re-exports if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/openhuman/billing/tools.rs`:
- Around line 205-214: The parameters_schema() function currently allows
amount_usd to be 0 by using "minimum": 0; update the schema for the "amount_usd"
property in parameters_schema to enforce a positive top-up (e.g., replace
"minimum": 0 with either "exclusiveMinimum": 0 to permit fractional positives or
set "minimum": 0.01 or "minimum": 1 if whole dollars only) so that zero-value
top-ups are rejected.
---
Nitpick comments:
In `@src/openhuman/billing/tools.rs`:
- Around line 222-225: Replace the current extraction of amount_usd as an f64
with a Decimal-based parse: read args.get("amount_usd") into a
rust_decimal::Decimal (accepting numeric or string JSON values), multiply by 100
and round to derive integer cents (u64) for internal representation (use a local
name like amount_cents or amount_usd_decimal), and then when calling
billing::top_up_credits convert explicitly from the Decimal or cents to the API
type if needed; update uses of the old local symbol amount to the new symbol
(e.g., amount_usd_decimal or amount_cents) and keep the conversion to f64 only
at the final API boundary to minimize precision loss (refer to
args.get("amount_usd") extraction and the billing::top_up_credits call).
In `@src/openhuman/cost/tools.rs`:
- Around line 87-93: The days extraction in async fn execute uses
.as_u64().map(|v| v as u32) which can silently truncate values > u32::MAX;
update the extraction to validate bounds before casting (e.g., check the u64
against u32::MAX and against a sensible application maximum like 365 or return
an error), then safely convert (using u32::try_from or an explicit check) and
use the validated/capped value for days.
In `@src/openhuman/learning/tools.rs`:
- Around line 41-43: The helper facet_to_json currently swallows
serde_json::to_value errors by returning Value::Null, which hides serialization
failures for ProfileFacet; update facet_to_json to return a
Result<serde_json::Value, serde_json::Error> (or Option with logging) so callers
can handle errors, or at minimum log the serde_json error before defaulting to
Null; locate the function facet_to_json and change its signature and
implementation to propagate the serde_json::Error (or call
serde_json::to_value(f).map_err(|e| { log::warn!(...); e })) and update all call
sites that use facet_to_json to handle the Result/Option accordingly.
In `@src/openhuman/task_sources/tools.rs`:
- Around line 1-571: Split the large module by moving the read-only tools
(TaskSourceListTool, TaskSourceGetTool, TaskSourceFetchTool,
TaskSourceListTasksTool, TaskSourcePreviewFilterTool, TaskSourceStatusTool) into
a new submodule (e.g., tools::read) and the mutator tools (TaskSourceAddTool,
TaskSourceUpdateTool, TaskSourceRemoveTool) into a new submodule (e.g.,
tools::write); keep the shared helpers and macro (read_required_str, opt_u64,
opt_str, parse_provider, parse_filter, emit!) in the current module (or a small
shared submodule) with pub(crate) visibility so submodules can use them, then
re-export the tool types and constructors from the parent module so external
callers keep the same names, and update the tests to reference the re-exports if
needed.
In `@src/openhuman/team/tools.rs`:
- Around line 41-70: The macro cfg_read! currently generates Tool impls with no
diagnostics; add stable, grep-friendly debug/trace logging (using tracing or
log) around construction and execution: instrument the $ty::new (or inside impl
Tool::execute) to emit a trace/debug before and after calling
emit!(team::$fn(&self.config).await, $name) and log the arguments/tool name and
result/error, and apply the same pattern to the other macros team_id_read! and
team_id_mutator! so each generated Tool logs entry, exit, and errors with a
consistent prefix that includes $name and the invoked function (team::$fn).
In `@src/openhuman/todos/tools.rs`:
- Around line 1-602: The module is over the ~500-line guideline; split
src/openhuman/todos/tools.rs by moving tool implementations into submodules
(e.g., move TodoListTool into tools/read.rs; TodoAddTool, TodoEditTool,
TodoUpdateStatusTool, TodoDecidePlanTool into tools/write.rs; TodoRemoveTool,
TodoReplaceTool, TodoClearTool into tools/destructive.rs) and keep shared
helpers (board_location, card_patch, read_required_str, opt_str, opt_str_vec,
snapshot_to_result, thread_id_prop) plus common imports in tools/mod.rs;
re-export the tools from mod.rs (pub use
crate::openhuman::todos::tools::{TodoListTool, TodoAddTool, ...}) so external
callers keep the same names, and update tests to reference the new module layout
or move test blocks into the appropriate submodule files.
In `@src/openhuman/tools/ops.rs`:
- Around line 83-753: The all_tools_with_runtime function is too large; extract
the five theme-based registration blocks into small helper builders (e.g.
register_knowledge_tools, register_productivity_tools, register_system_tools,
register_account_tools, register_desktop_tools) that return Vec<Box<dyn Tool>>
and then call tools.extend(...) from all_tools_with_runtime; each helper should
accept only the specific parameters it needs (e.g. config: Arc<Config>,
security: Arc<SecurityPolicy>, memory: Arc<dyn Memory>, root_config clone,
workspace_dir, runtime/audit as needed) and construct the same Box::new(...)
entries you moved (e.g. VaultListTool::new, SkillListTool::new, ThreadListTool,
LearningListFacetsTool, BillingPlanTool::new, ScreenStatusTool, etc.), then
replace the original inline blocks in all_tools_with_runtime with
tools.extend(helper(...)) to preserve behavior and keep node/browser/integration
gating unchanged.
In `@src/openhuman/vault/tools.rs`:
- Around line 27-52: The helpers read_required_str, opt_str_vec and the emit!
macro are duplicated across multiple tool modules; extract them into a shared
module (e.g., crate::openhuman::tools::helpers) and replace local definitions
with imports. Move the implementations of read_required_str, opt_str_vec and
emit! into the new helpers module, export them (pub) and update callers in
vault/tools.rs (and people, skills, service tool files) to use
crate::openhuman::tools::helpers::{read_required_str, opt_str_vec, emit}. Ensure
the emit! macro signature and error-wrapping behavior remain identical so
existing call sites (which expect
ToolResult::success(serde_json::to_string(&outcome.value)?)) continue to work.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 21168f40-f886-4f8a-9edd-7fa41f66dbc5
📒 Files selected for processing (50)
src/openhuman/agent_workflows/mod.rssrc/openhuman/agent_workflows/tools.rssrc/openhuman/artifacts/mod.rssrc/openhuman/artifacts/tools.rssrc/openhuman/billing/mod.rssrc/openhuman/billing/tools.rssrc/openhuman/config/mod.rssrc/openhuman/config/tools.rssrc/openhuman/cost/mod.rssrc/openhuman/cost/tools.rssrc/openhuman/credentials/mod.rssrc/openhuman/credentials/tools.rssrc/openhuman/dashboard/mod.rssrc/openhuman/dashboard/tools.rssrc/openhuman/doctor/mod.rssrc/openhuman/doctor/tools.rssrc/openhuman/health/mod.rssrc/openhuman/health/tools.rssrc/openhuman/learning/mod.rssrc/openhuman/learning/tools.rssrc/openhuman/mcp_registry/mod.rssrc/openhuman/mcp_registry/tools.rssrc/openhuman/people/mod.rssrc/openhuman/people/tools.rssrc/openhuman/referral/mod.rssrc/openhuman/referral/tools.rssrc/openhuman/screen_intelligence/mod.rssrc/openhuman/screen_intelligence/tools.rssrc/openhuman/security/mod.rssrc/openhuman/security/tools.rssrc/openhuman/service/mod.rssrc/openhuman/service/tools.rssrc/openhuman/skills/mod.rssrc/openhuman/skills/tools.rssrc/openhuman/task_sources/mod.rssrc/openhuman/task_sources/tools.rssrc/openhuman/team/mod.rssrc/openhuman/team/tools.rssrc/openhuman/threads/mod.rssrc/openhuman/threads/tools.rssrc/openhuman/todos/mod.rssrc/openhuman/todos/tools.rssrc/openhuman/tools/mod.rssrc/openhuman/tools/ops.rssrc/openhuman/tools/ops_tests.rssrc/openhuman/tools/user_filter.rssrc/openhuman/vault/mod.rssrc/openhuman/vault/tools.rssrc/openhuman/workspace/mod.rssrc/openhuman/workspace/tools.rs
Comment on lines
+205
to
+214
| fn parameters_schema(&self) -> serde_json::Value { | ||
| json!({ | ||
| "type": "object", | ||
| "properties": { | ||
| "amount_usd": { "type": "number", "minimum": 0 }, | ||
| "gateway": { "type": "string" } | ||
| }, | ||
| "required": ["amount_usd"] | ||
| }) | ||
| } |
Contributor
There was a problem hiding this comment.
Schema allows zero for amount_usd, which is likely invalid for a top-up.
The parameter schema sets "minimum": 0 for the top-up amount, which would allow topping up $0.00. This is nonsensical and should be rejected.
🛡️ Set a positive minimum value
"properties": {
- "amount_usd": { "type": "number", "minimum": 0 },
+ "amount_usd": { "type": "number", "minimum": 0.01 },
"gateway": { "type": "string" }
},Or use a higher minimum like 1 if fractional dollars aren't supported.
📝 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 parameters_schema(&self) -> serde_json::Value { | |
| json!({ | |
| "type": "object", | |
| "properties": { | |
| "amount_usd": { "type": "number", "minimum": 0 }, | |
| "gateway": { "type": "string" } | |
| }, | |
| "required": ["amount_usd"] | |
| }) | |
| } | |
| fn parameters_schema(&self) -> serde_json::Value { | |
| json!({ | |
| "type": "object", | |
| "properties": { | |
| "amount_usd": { "type": "number", "minimum": 0.01 }, | |
| "gateway": { "type": "string" } | |
| }, | |
| "required": ["amount_usd"] | |
| }) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/openhuman/billing/tools.rs` around lines 205 - 214, The
parameters_schema() function currently allows amount_usd to be 0 by using
"minimum": 0; update the schema for the "amount_usd" property in
parameters_schema to enforce a positive top-up (e.g., replace "minimum": 0 with
either "exclusiveMinimum": 0 to permit fractional positives or set "minimum":
0.01 or "minimum": 1 if whole dollars only) so that zero-value top-ups are
rejected.
Resolve conflict with tinyhumansai#3040 (drop Knowledge Vaults): upstream removed the entire vault backend domain, so drop the vault agent tools (vault_list/get/ files/create/sync/sync_status/remove) and all their wiring (tools/mod.rs re-export, ops.rs registrations, user_filter vault_remove toggle, and the knowledge-theme e2e vault entries + vault_list execution test). Remaining agent-tool expansion is unaffected; lib+tests compile clean and all expansion tests pass.
Member
Author
|
Rebased onto latest Note: the pre-push hook flagged ESLint |
Merged
12 tasks
senamakel
added a commit
to senamakel/openhuman
that referenced
this pull request
Jun 6, 2026
…nding tools default-OFF) (tinyhumansai#3050)
Merged
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Exposes ~160 new agent tools across 23 core domains that previously had RPC/ops surfaces but no agent-callable
Toolwrappers. Each tool is a thin shim over an existingops/rpcfunction — no new business logic, no new RPC. Tools are registered intools::ops::all_tools_with_runtimeand re-exported viatools/mod.rs.The guiding rule: tools that overextend ship default-OFF. Read-only and bounded/reversible writers are default-ON; destructive deletes, privilege escalation, secret exposure, external money/comms, and system-state mutation are made filterable in
tools/user_filter.rsunder per-family toggle IDs (mirroring the existingupdate_*opt-in precedent) so onboarding can default them off. Every tool also sets a per-callPermissionLevel(ReadOnly → Dangerous).Themes & tools
Default-OFF toggle families (in
user_filter.rs)agent_workflow_uninstall·artifact_delete·todo_destructive·task_source_manage·vault_remove·people_refresh_address_book·skill_manage·thread_destructive·learning_manage·service_lifecycle·billing_writes·team_admin·screen_permissions·mcp_manage·workspace_manageDeliberately deferred (no clean backing — documented in-code)
config_update_*mutators — apply fns take hand-built non-Deserialize*SettingsPatchstructs mapped field-by-field from separate wire types; needs those wire types made public (or aconfig::opspatch-from-json helper). Config reads are included.filter, dashboardget_settings, security command/path/rate checks, configget_analytics/get_meet— no backing RPC exists; not invented.provider_surface_update_status— needs a new store mutator (out of scope for a wrapper-only PR).Tests
tools/ops_tests.rsdriving the fullall_toolsregistry per theme: every tool registers, default-OFF tools are stripped byfilter_tools_by_user_preferencewhen not opted in and restored when opted in, plus real executions through the boxeddyn Toolsurface (todo_add→todo_list,artifact_list,vault_list,health_system_info).cargo fmt --checkclean;cargo check --lib --testsclean; all new tests pass.Notes
artifactshas no producer yet (save_artifact_metais unused), soartifact_listreturns empty until a generator domain populates it — the tools are correct, the feed is dormant.🤖 Generated with Claude Code
Summary by CodeRabbit