feat: make autonomy action budget configurable#2499
Open
YOMXXX wants to merge 5 commits into
Open
Conversation
Contributor
|
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}
Contributor
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
Contributor
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
Contributor
Author
|
@graycyrus CI is green and CodeRabbit is approved. Could you take a look when you have a chance? |
Contributor
Author
|
@graycyrus Ready for review again. Latest state on
|
Contributor
Author
|
@graycyrus Ready for review again. Latest state after syncing current
|
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
max_actions_per_hour, including a typed config patch, config load/apply helper, and controller/RPC exposure.OPENHUMAN_MAX_ACTIONS_PER_HOURenv override documentation/defaults for local and deployment tuning.Problem
security_policy_infoused default policy data, which could drift from the active runtime config.Solution
AutonomySettingsPatchandapply_autonomy_settings/load_and_apply_autonomy_settingsso only supported autonomy settings are mutated and persisted.config.update_autonomy_settings/openhuman.config_update_autonomy_settingsthrough the controller path rather than adding adapter-specific logic.ActionBudgetPanelunder Settings -> Advanced with validation, save state, localized copy, and focused tests.SecurityPolicy.OPENHUMAN_MAX_ACTIONS_PER_HOURand documented it in.env.exampleplus the agent harness docs.openhuman.update_autonomy_settings.Submission Checklist
diff-cover) meet the gate enforced by.github/workflows/coverage.yml. Runpnpm test:coverageandpnpm test:rustlocally; PRs below 80% on changed lines will not merge. Localpnpm test:coveragepassed; Rust focused tests passed; CI diff coverage 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 backend auth/socket feedback issues; this PR addresses the configurable action-budget portion only.Impact
OPENHUMAN_MAX_ACTIONS_PER_HOUR.Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
feat/configurable-action-budget937fb4bbed877c631a4d1bf548895bce70489fb2Validation Run
pnpm --filter openhuman-app format:checkviapnpm format:checkpnpm typecheckviapnpm compilepnpm --filter openhuman-app exec vitest run --config test/vitest.config.ts src/components/settings/panels/__tests__/ActionBudgetPanel.test.tsx src/components/settings/panels/__tests__/DeveloperOptionsPanel.test.tsxpnpm test:coverage(319 test files passed, 3096 tests passed, 3 skipped)pnpm format:check;GGML_NATIVE=OFF cargo test --manifest-path Cargo.toml frontend_legacy_aliases_match_server_alias_table;GGML_NATIVE=OFF cargo test --manifest-path Cargo.toml action_budget;GGML_NATIVE=OFF cargo test --manifest-path Cargo.toml env_overlay_autonomy_max_actions_per_hour_accepts_valid_u32;GGML_NATIVE=OFF cargo test --manifest-path Cargo.toml security_policy_info;GGML_NATIVE=OFF cargo check --manifest-path Cargo.tomlGGML_NATIVE=OFF pnpm rust:checkpnpm i18n:check;pnpm lint(passes with 44 existing warnings);git diff --check; latest pre-push hook passed withgit push origin feat/configurable-action-budgeton commit937fb4bb.Validation Blocked
command:GGML_NATIVE=OFF pnpm test:rusterror:Local full Rust suite ran 8530 passing tests before failing 6 tests. One real drift failure (frontend_legacy_aliases_match_server_alias_table) was fixed and revalidated. The remaining 5 failures are local network/runtime expectation mismatches where connection-refused tests received502 Bad Gatewayfrom local OpenAI/Ollama-style requests.impact:Changed-surface Rust tests, alias drift guard, frontend coverage, Tauri check, and pre-push passed locally; CI Rust/coverage jobs are the authoritative full-suite gate.Notes:
v22.14.0while the app declares>=24.0.0; pnpm emitted engine warnings but the commands above exited successfully unless noted in Validation Blocked.whisper-rsneeds the documentedGGML_NATIVE=OFFworkaround to avoid-mcpu=nativefailures.Behavior Changes
Parity Contract
Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
OPENHUMAN_MAX_ACTIONS_PER_HOURenvironment variable (default: 20) for server-side rate limiting configuration.Documentation