feat: make autonomy action budget configurable#2499
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR implements a rolling-hour action budget feature that allows administrators to cap side-effecting tool operations. Changes span backend config/RPC/security updates, a new frontend settings panel with Tauri integration, routing wiring, comprehensive i18n additions across all language chunks, environment variable override support, and architecture documentation. ChangesAction Budget Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
coderabbitai
Bot
added
the
feature
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/openhuman/config/schemas.rs (1)
1022-1030: ⚡ Quick winAdd RPC entry/exit/error logs to the new autonomy update handler.
This path mutates persisted config and currently has no diagnostic logs for enter/success/failure.
Suggested patch
fn handle_update_autonomy_settings(params: Map<String, Value>) -> ControllerFuture { Box::pin(async move { - let update = deserialize_params::<AutonomySettingsUpdate>(params)?; + log::debug!("[config][rpc] update_autonomy_settings enter"); + let update = match deserialize_params::<AutonomySettingsUpdate>(params) { + Ok(u) => u, + Err(err) => { + log::warn!("[config][rpc] update_autonomy_settings invalid params: {err}"); + return Err(err); + } + }; let patch = config_rpc::AutonomySettingsPatch { max_actions_per_hour: update.max_actions_per_hour, }; - to_json(config_rpc::load_and_apply_autonomy_settings(patch).await?) + match config_rpc::load_and_apply_autonomy_settings(patch).await { + Ok(outcome) => { + log::debug!("[config][rpc] update_autonomy_settings ok"); + to_json(outcome) + } + Err(err) => { + log::warn!("[config][rpc] update_autonomy_settings failed: {err}"); + Err(err) + } + } }) }As per coding guidelines:
Use log / tracing at debug or trace level on RPC entry and exit, error paths, state transitions....🤖 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/config/schemas.rs` around lines 1022 - 1030, Add diagnostic logs to handle_update_autonomy_settings: on RPC entry log the incoming params/deserialized update (debug/trace), wrap the call to config_rpc::load_and_apply_autonomy_settings(patch).await with a success log on completion (debug) and an error log that captures the error context before returning (error). Ensure logs include the function name handle_update_autonomy_settings and the patch/max_actions_per_hour value and propagate the original error unchanged after logging.src/openhuman/security/schemas.rs (1)
42-45: ⚡ Quick winAdd diagnostic logging around policy_info RPC execution.
This RPC now depends on async config I/O but doesn’t log entry/success/failure, which makes field debugging harder.
Suggested patch
fn handle_policy_info(_params: Map<String, Value>) -> ControllerFuture { - Box::pin(async { - to_json(crate::openhuman::security::rpc::load_and_get_security_policy_info().await?) + Box::pin(async { + log::debug!("[security][rpc] policy_info enter"); + match crate::openhuman::security::rpc::load_and_get_security_policy_info().await { + Ok(outcome) => { + log::debug!("[security][rpc] policy_info ok"); + to_json(outcome) + } + Err(err) => { + log::warn!("[security][rpc] policy_info failed: {err}"); + Err(err) + } + } }) }As per coding guidelines:
Use log / tracing at debug or trace level on RPC entry and exit, error paths, state transitions....🤖 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/security/schemas.rs` around lines 42 - 45, The RPC handler handle_policy_info lacks diagnostics; add tracing/log calls around the async call to crate::openhuman::security::rpc::load_and_get_security_policy_info() to record entry, success, and failure: log at debug/trace when entering handle_policy_info (include any incoming params or a brief marker), log debug/trace on successful result before returning the JSON, and log error with the error details if the await returns Err (include context string). Use the existing logging/tracing crate conventions (e.g., tracing::debug!/tracing::error!) so callers can trace async config I/O and failures in load_and_get_security_policy_info().
🤖 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 `@app/src/components/settings/panels/ActionBudgetPanel.tsx`:
- Around line 47-51: The panel currently shows a desktop-only error but still
allows Save to be clicked; prevent that by gating the save path and disabling
the Save button when not running in Tauri: add a runtime flag check (use
isTauri() or a local isTauriState set on mount) at the top of the save handler
(e.g., the component's save/handleSave function) to return early when not Tauri,
and set the Save button's disabled prop to true when !isTauri() (also update the
other occurrence referenced around lines 147-151). Ensure you use the same
isTauri/isTauriState used where setIsLoading and setError are called so UI state
and action gating remain consistent.
---
Nitpick comments:
In `@src/openhuman/config/schemas.rs`:
- Around line 1022-1030: Add diagnostic logs to handle_update_autonomy_settings:
on RPC entry log the incoming params/deserialized update (debug/trace), wrap the
call to config_rpc::load_and_apply_autonomy_settings(patch).await with a success
log on completion (debug) and an error log that captures the error context
before returning (error). Ensure logs include the function name
handle_update_autonomy_settings and the patch/max_actions_per_hour value and
propagate the original error unchanged after logging.
In `@src/openhuman/security/schemas.rs`:
- Around line 42-45: The RPC handler handle_policy_info lacks diagnostics; add
tracing/log calls around the async call to
crate::openhuman::security::rpc::load_and_get_security_policy_info() to record
entry, success, and failure: log at debug/trace when entering handle_policy_info
(include any incoming params or a brief marker), log debug/trace on successful
result before returning the JSON, and log error with the error details if the
await returns Err (include context string). Use the existing logging/tracing
crate conventions (e.g., tracing::debug!/tracing::error!) so callers can trace
async config I/O and failures in load_and_get_security_policy_info().
🪄 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: 9f524bd5-1e23-415d-bda2-0e63ff611f21
📒 Files selected for processing (32)
.env.exampleapp/src/components/settings/panels/ActionBudgetPanel.tsxapp/src/components/settings/panels/DeveloperOptionsPanel.tsxapp/src/components/settings/panels/__tests__/ActionBudgetPanel.test.tsxapp/src/lib/i18n/chunks/ar-5.tsapp/src/lib/i18n/chunks/bn-5.tsapp/src/lib/i18n/chunks/de-3.tsapp/src/lib/i18n/chunks/de-5.tsapp/src/lib/i18n/chunks/en-5.tsapp/src/lib/i18n/chunks/es-5.tsapp/src/lib/i18n/chunks/fr-5.tsapp/src/lib/i18n/chunks/hi-5.tsapp/src/lib/i18n/chunks/id-5.tsapp/src/lib/i18n/chunks/it-5.tsapp/src/lib/i18n/chunks/ko-5.tsapp/src/lib/i18n/chunks/pt-5.tsapp/src/lib/i18n/chunks/ru-5.tsapp/src/lib/i18n/chunks/zh-CN-5.tsapp/src/lib/i18n/en.tsapp/src/pages/Settings.tsxapp/src/services/rpcMethods.tsapp/src/utils/tauriCommands/config.tsgitbooks/developing/architecture/agent-harness.mdsrc/openhuman/config/ops.rssrc/openhuman/config/ops_tests.rssrc/openhuman/config/schema/load.rssrc/openhuman/config/schema/load_tests.rssrc/openhuman/config/schemas.rssrc/openhuman/security/ops.rssrc/openhuman/security/policy.rssrc/openhuman/security/policy_tests.rssrc/openhuman/security/schemas.rs
💤 Files with no reviewable changes (1)
- app/src/lib/i18n/chunks/de-3.ts
YOMXXX
force-pushed
the
feat/configurable-action-budget
branch
from
497d027 to
2dac605
Compare
YOMXXX
force-pushed
the
feat/configurable-action-budget
branch
from
2dac605 to
c68a3ab
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/openhuman/security/schemas.rs (1)
42-45: ⚡ Quick winInstrument
policy_infohandler with debug/warn logs.The new async path is good, but it currently has no explicit RPC entry/failure logs for observability.
As per coding guidelines: `src/openhuman/**/*.rs` should log RPC entry/exit and error paths with structured, grep-friendly context.Suggested update
fn handle_policy_info(_params: Map<String, Value>) -> ControllerFuture { Box::pin(async { - to_json(crate::openhuman::security::rpc::load_and_get_security_policy_info().await?) + log::debug!("[security][rpc] policy_info enter"); + match crate::openhuman::security::rpc::load_and_get_security_policy_info().await { + Ok(outcome) => { + log::debug!("[security][rpc] policy_info ok"); + to_json(outcome) + } + Err(err) => { + log::warn!("[security][rpc] policy_info failed: {err}"); + Err(err) + } + } }) }🤖 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/security/schemas.rs` around lines 42 - 45, Instrument the handle_policy_info RPC handler to emit structured entry/exit and error logs: at the start of handle_policy_info() log RPC entry with a context field like {"rpc":"policy_info"}, then wrap the await call to crate::openhuman::security::rpc::load_and_get_security_policy_info() so that on success you log a debug/trace exit (include any lightweight result metadata) and on failure you log a warn/error with the error details and same context; use the project logging macros (e.g., tracing::debug!/warn!) and ensure logs reference the function name handle_policy_info and the called function load_and_get_security_policy_info for easy grepping.src/openhuman/config/schemas.rs (1)
1022-1030: ⚡ Quick winAdd entry/success/failure logs to the new autonomy RPC handler.
This new RPC path currently has no debug/warn instrumentation, which makes runtime triage harder than adjacent handlers.
As per coding guidelines: `src/openhuman/**/*.rs`: use `log` / `tracing` at `debug` or `trace` on RPC entry/exit and error paths with grep-friendly context.Proposed logging pattern
fn handle_update_autonomy_settings(params: Map<String, Value>) -> ControllerFuture { Box::pin(async move { - let update = deserialize_params::<AutonomySettingsUpdate>(params)?; + log::debug!("[config][rpc] update_autonomy_settings enter"); + let update = match deserialize_params::<AutonomySettingsUpdate>(params) { + Ok(u) => u, + Err(err) => { + log::warn!("[config][rpc] update_autonomy_settings invalid params: {err}"); + return Err(err); + } + }; let patch = config_rpc::AutonomySettingsPatch { max_actions_per_hour: update.max_actions_per_hour, }; - to_json(config_rpc::load_and_apply_autonomy_settings(patch).await?) + match config_rpc::load_and_apply_autonomy_settings(patch).await { + Ok(outcome) => { + log::debug!("[config][rpc] update_autonomy_settings ok"); + to_json(outcome) + } + Err(err) => { + log::warn!("[config][rpc] update_autonomy_settings failed: {err}"); + Err(err) + } + } }) }🤖 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/config/schemas.rs` around lines 1022 - 1030, The handler handle_update_autonomy_settings lacks logging; add trace/debug logs on entry (log the deserialized AutonomySettingsUpdate or its key fields), a success debug log after to_json returns (including the applied patch or resulting state), and an error/warn log around the await of config_rpc::load_and_apply_autonomy_settings(patch).await? so any failure is logged with the error details and a grep-friendly context string like "handle_update_autonomy_settings". Use the project's logging crate (log or tracing) consistent with nearby handlers and include the function name and relevant identifiers (AutonomySettingsUpdate, AutonomySettingsPatch, config_rpc::load_and_apply_autonomy_settings) in messages.
🤖 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 @.env.example:
- Around line 75-76: Move the OPENHUMAN_MAX_ACTIONS_PER_HOUR entry so it appears
before the OPENHUMAN_MODEL entry in the .env example to satisfy dotenv key
ordering; update the block by cutting the line
"OPENHUMAN_MAX_ACTIONS_PER_HOUR=20" and pasting it above the existing
"OPENHUMAN_MODEL" key so dotenv-linter no longer reports UnorderedKey.
---
Nitpick comments:
In `@src/openhuman/config/schemas.rs`:
- Around line 1022-1030: The handler handle_update_autonomy_settings lacks
logging; add trace/debug logs on entry (log the deserialized
AutonomySettingsUpdate or its key fields), a success debug log after to_json
returns (including the applied patch or resulting state), and an error/warn log
around the await of config_rpc::load_and_apply_autonomy_settings(patch).await?
so any failure is logged with the error details and a grep-friendly context
string like "handle_update_autonomy_settings". Use the project's logging crate
(log or tracing) consistent with nearby handlers and include the function name
and relevant identifiers (AutonomySettingsUpdate, AutonomySettingsPatch,
config_rpc::load_and_apply_autonomy_settings) in messages.
In `@src/openhuman/security/schemas.rs`:
- Around line 42-45: Instrument the handle_policy_info RPC handler to emit
structured entry/exit and error logs: at the start of handle_policy_info() log
RPC entry with a context field like {"rpc":"policy_info"}, then wrap the await
call to crate::openhuman::security::rpc::load_and_get_security_policy_info() so
that on success you log a debug/trace exit (include any lightweight result
metadata) and on failure you log a warn/error with the error details and same
context; use the project logging macros (e.g., tracing::debug!/warn!) and ensure
logs reference the function name handle_policy_info and the called function
load_and_get_security_policy_info for easy grepping.
🪄 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: 5edfbc0e-0f78-4a08-a028-7fc9db5cfde4
📒 Files selected for processing (33)
.env.exampleapp/src/components/settings/panels/ActionBudgetPanel.tsxapp/src/components/settings/panels/DeveloperOptionsPanel.tsxapp/src/components/settings/panels/__tests__/ActionBudgetPanel.test.tsxapp/src/lib/i18n/chunks/ar-5.tsapp/src/lib/i18n/chunks/bn-5.tsapp/src/lib/i18n/chunks/de-3.tsapp/src/lib/i18n/chunks/de-5.tsapp/src/lib/i18n/chunks/en-5.tsapp/src/lib/i18n/chunks/es-5.tsapp/src/lib/i18n/chunks/fr-5.tsapp/src/lib/i18n/chunks/hi-5.tsapp/src/lib/i18n/chunks/id-5.tsapp/src/lib/i18n/chunks/it-5.tsapp/src/lib/i18n/chunks/ko-5.tsapp/src/lib/i18n/chunks/pt-5.tsapp/src/lib/i18n/chunks/ru-5.tsapp/src/lib/i18n/chunks/zh-CN-5.tsapp/src/lib/i18n/en.tsapp/src/pages/Settings.tsxapp/src/services/rpcMethods.tsapp/src/utils/tauriCommands/config.tsgitbooks/developing/architecture/agent-harness.mdsrc/core/legacy_aliases.rssrc/openhuman/config/ops.rssrc/openhuman/config/ops_tests.rssrc/openhuman/config/schema/load.rssrc/openhuman/config/schema/load_tests.rssrc/openhuman/config/schemas.rssrc/openhuman/security/ops.rssrc/openhuman/security/policy.rssrc/openhuman/security/policy_tests.rssrc/openhuman/security/schemas.rs
💤 Files with no reviewable changes (1)
- app/src/lib/i18n/chunks/de-3.ts
✅ Files skipped from review due to trivial changes (5)
- src/openhuman/security/policy.rs
- app/src/lib/i18n/chunks/es-5.ts
- app/src/lib/i18n/chunks/id-5.ts
- app/src/lib/i18n/chunks/en-5.ts
- app/src/lib/i18n/chunks/fr-5.ts
|
@graycyrus CI is green and CodeRabbit is approved. Could you take a look when you have a chance? |
|
@graycyrus Ready for review again. Latest state on
|
|
@graycyrus Ready for review again. Latest state after syncing current
|
# Conflicts: # app/src/lib/i18n/en.ts # app/src/pages/Settings.tsx
…sai#2500 landed PR tinyhumansai#2500 merged into main introduced an `AutonomyPanel` plus matching `update_autonomy_settings` RPC, `AutonomySettingsPatch` type, and `apply_autonomy_settings` ops that overlap with this PR's `ActionBudgetPanel`. The post-merge tree had: - two `openhumanUpdateAutonomySettings` TS functions (TS2323), - two `AutonomySettingsPatch` structs and two `apply_autonomy_settings` / `load_and_apply_autonomy_settings` fns in core (E0428/E0119), - two `AutonomySettingsUpdate` deserialize structs, two schema arms, two registered controllers, and two `handle_update_autonomy_settings` handlers in `config/schemas.rs`. Keep main's versions (they add `max_actions_per_hour` range validation and ship a sibling `get_autonomy_settings` RPC) and drop the PR's duplicates. Both UI panels (`AutonomyPanel`, `ActionBudgetPanel`) now share the same backend. Note: `AutonomyPanel` and `ActionBudgetPanel` still both render in DeveloperOptions for the same setting — flagging for human decision in a follow-up PR comment.
|
@graycyrus @YOMXXX heads-up after re-syncing PR #2500 ("feat(app): UI control for max_actions_per_hour") merged into main while this PR was open and ships effectively the same feature under a different name. After merging
I deduped on the backend by keeping main's versions (they add
Latest pushed commit: ec55001 |
# Conflicts: # app/src-tauri/Cargo.lock
|
@graycyrus @senamakel Updated in |
|
@graycyrus @senamakel Ready for review again. Latest state on
|
|
Checked the latest CI. The blocking issue is the same Linux Appium E2E failure seen on #2551:
The Windows secrets ACL check is reported as failed in the checks table, but GitHub marks the job conclusion as
|
|
Follow-up: opened #2609 to make this shared E2E failure diagnosable. It adds runner-temp app log upload paths and changes the mega-flow Composio RPC assertions to include the RPC method/status/error instead of only This should also help distinguish the Linux Appium Composio blocker from the separate Windows secrets ACL |
|
Follow-up on the shared Linux Appium failure: #2609 now confirms and fixes the root cause. The failing #2609 defaults E2E sessions to |
2 participants
Summary
OPENHUMAN_MAX_ACTIONS_PER_HOURas an env overlay forconfig.autonomy.max_actions_per_hour.openhuman.security_policy_inforeflect the active loaded config instead ofSecurityPolicy::default().openhuman.update_autonomy_settingsalias on both frontend and core routing.ActionBudgetPanelUI/route/i18n so users do not see two panels editing the same setting.Problem
max_actions_per_hourUI/RPC feature asAutonomyPanelat/settings/autonomy.ActionBudgetPanelat/settings/action-budget, which duplicated the same config field in Developer Options.Solution
max_actions_per_hour.ActionBudgetPanel, its route, menu item, test, and i18n strings from this branch.openhuman.update_autonomy_settings.Submission Checklist
diff-cover) meet the gate enforced by.github/workflows/coverage.yml. Focused Rust/Vitest tests cover the changed paths; CI Coverage Gate remains authoritative.docs/TEST-COVERAGE-MATRIX.md.## Related- N/A: no matrix feature ID changed.Closes #NNNin the## Relatedsection - N/A: OpenHuman 不执行命令和无反馈的分析 #2486 also covers command allowlists, approval UX, auth/socket feedback, and no-feedback diagnostics; this PR addresses the action-budget backend/operator portion only.Impact
OPENHUMAN_MAX_ACTIONS_PER_HOUR.security_policy_infonow reports policy values derived from the active config.Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
feat/configurable-action-budget472008bf6ee6168a203824cb960679ab54cf1183Validation Run
pnpm --filter openhuman-app format:checkpnpm typecheck(afterpnpm install --frozen-lockfileinstalled branch-added workspace deps)pnpm --filter openhuman-app test:unit src/components/settings/panels/__tests__/DeveloperOptionsPanel.test.tsx src/components/settings/panels/__tests__/AutonomyPanel.test.tsx;GGML_NATIVE=OFF cargo test --manifest-path Cargo.toml action_budget_error_mentions_limit_and_settings --lib;GGML_NATIVE=OFF cargo test --manifest-path Cargo.toml env_overlay_autonomy_max_actions_per_hour_accepts_valid_u32 --lib;GGML_NATIVE=OFF cargo test --manifest-path Cargo.toml security_policy_info_reflects_configured_action_budget --libcargo fmt --manifest-path Cargo.toml --all --check; pre-pushcargo check --manifest-path app/src-tauri/Cargo.tomlcompleted successfully.pnpm rust:checkcompleted successfully.pnpm i18n:check;git diff --check; pre-push hook completed successfully on commit472008bf(format:check,lintwith existing warnings only,compile,rust:check,lint:commands-tokens).Validation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
Parity Contract
openhuman.update_autonomy_settingsaliases map toopenhuman.config_update_autonomy_settingson frontend and core.Duplicate / Superseded PR Handling
max_actions_per_hourUI/RPC feature while this PR was open.472008bfto remove the duplicateActionBudgetPaneland keep only non-overlapping backend/operator value.