fix(settings): tool capability toggles now persist and are enforced at runtime#720
Conversation
… handling - Bumped the OpenHuman package version to 0.52.26 in Cargo.lock files. - Enhanced the ToolsPanel component to include user feedback on save status. - Implemented filtering of tools based on user preferences in the Rust backend, allowing for more customizable tool availability. - Added new utility functions to map UI tool IDs to Rust tool names for better integration. These changes improve user experience and maintain compatibility with the latest features.
📝 WalkthroughWalkthroughThis PR implements tool capability persistence for the Settings interface. Users can now toggle tools on/off, save preferences to app state, and have those preferences applied during agent session building. The solution includes UI-to-Rust tool name mapping, visual save feedback, and prevents race conditions during state updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as ToolsPanel<br/>(Frontend)
participant Service as chatService<br/>(Save)
participant AppState as App State<br/>(Persistence)
participant Builder as Session Builder<br/>(Backend)
participant Agent as Agent<br/>(Tool Execution)
User->>UI: Toggle tool on/off
UI->>UI: Update local state (enabledIds)
User->>UI: Click "Save Changes"
UI->>UI: Map IDs to Rust names<br/>via getEnabledRustToolNames()
UI->>Service: setOnboardingTasks({enabledTools})
Service->>AppState: Persist enabled_tools list
AppState->>AppState: Write to storage
UI->>UI: Set saveStatus='saved'<br/>(auto-reset after delay)
User->>Builder: Start new agent session
Builder->>AppState: load_stored_app_state()
AppState->>Builder: Return enabled_tools list
Builder->>Builder: filter_tools_by_user_preference()<br/>(retain only enabled + infrastructure)
Builder->>Agent: Provide filtered tools
Agent->>Agent: Execute with user-filtered tools
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/components/settings/panels/ToolsPanel.tsx (1)
30-41:⚠️ Potential issue | 🟠 MajorInitialization logic will break for multi-tool mappings after first save.
After
handleSave,onboardingTasks.enabledToolscontains Rust tool names (e.g.,["cron_add", "cron_list", ...]), not UI IDs. The checkenabledList.includes(tool.id)on line 37 compares against UI IDs like"cron", which won't match the persisted Rust names.This means after the first save, tools with multi-name mappings (cron, file_read, file_write) will appear disabled in the UI even though they're actually enabled on the backend.
🐛 Proposed fix: reverse-map Rust names back to UI IDs on load
+import { + CATEGORY_DESCRIPTIONS, + getDefaultEnabledTools, + getEnabledRustToolNames, + getToolsByCategory, + TOOL_CATALOG, + TOOL_CATEGORIES, +} from '../../../utils/toolDefinitions'; +// Build reverse mapping: Rust tool name → UI ID +const rustNameToUiId = new Map<string, string>(); +for (const tool of TOOL_CATALOG) { + for (const rustName of tool.rustToolNames) { + rustNameToUiId.set(rustName, tool.id); + } +} // Initialise toggle state from core state (persisted) or defaults. useEffect(() => { if (savingRef.current) return; const persisted = onboardingTasks?.enabledTools; - const enabledList = persisted && persisted.length > 0 ? persisted : getDefaultEnabledTools(); + let enabledUiIds: string[]; + if (persisted && persisted.length > 0) { + // Persisted list contains Rust names; convert back to UI IDs + const uiIdSet = new Set<string>(); + for (const rustName of persisted) { + const uiId = rustNameToUiId.get(rustName); + if (uiId) uiIdSet.add(uiId); + } + enabledUiIds = Array.from(uiIdSet); + } else { + enabledUiIds = getDefaultEnabledTools(); + } const map: Record<string, boolean> = {}; for (const cat of TOOL_CATEGORIES) { for (const tool of toolsByCategory[cat]) { - map[tool.id] = enabledList.includes(tool.id); + map[tool.id] = enabledUiIds.includes(tool.id); } } setEnabled(map); }, [onboardingTasks?.enabledTools]); // eslint-disable-line react-hooks/exhaustive-deps🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/settings/panels/ToolsPanel.tsx` around lines 30 - 41, The initialization in the useEffect compares persisted backend names (onboardingTasks.enabledTools) against UI tool IDs (tool.id) so multi-name tools appear disabled after save; fix by treating persisted list as backend names: build a Set from onboardingTasks.enabledTools and for each tool in toolsByCategory use a backend-name mapping (e.g., tool.backendNames or tool.rustNames — whichever property holds the Rust/backend identifiers) and mark the tool enabled if any of those backend names exist in the persisted Set; alternatively create a reverse map from backend name -> ui id once and check membership. Update the comparison in the useEffect (where enabledList.includes(tool.id) is used) to check backend name membership and then call setEnabled(map) as before.
🧹 Nitpick comments (2)
app/src/components/settings/panels/ToolsPanel.tsx (1)
22-25: Race condition guard withsavingRefis reasonable but the 500ms delay is somewhat arbitrary.The pattern works but consider:
- If the Redux refresh takes longer than 500ms (slow device, large state), the guard may clear too early
- If it takes less, users wait unnecessarily
A more robust approach would be to track a specific save "version" or use a flag that's cleared only when the effect actually runs with the new data.
Also applies to: 50-50, 75-77
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/settings/panels/ToolsPanel.tsx` around lines 22 - 25, The current 500ms timeout guard using savingRef is brittle; replace it by tracking a concrete save "version" or token instead: when performing the save increment a local saveVersionRef (or generate a unique saveToken) and set savingRef true and saveStatus accordingly, then in the useEffect that reacts to the Redux/settings state compare the incoming settings/version to that saveVersion/token and only clear savingRef and setSaveStatus('saved') when the effect observes the new version/token; reference and update savingRef, saveStatus/setSaveStatus and the existing useEffect that currently resets the guard so the guard is cleared deterministically when the new data actually arrives instead of after a fixed 500ms timeout.src/openhuman/tools/user_filter.rs (1)
6-32: Dual mapping maintenance burden.This mapping duplicates
rustToolNamesfromapp/src/utils/toolDefinitions.ts. If a new tool is added or a mapping changes, both files must be updated in sync. Consider adding a comment noting this dependency.📝 Suggested comment addition
/// Maps UI-level tool toggle IDs (stored in app state) to the Rust tool /// `name()` values they control. Tools not covered by any mapping entry /// are always retained — only tools that appear here are filterable. +/// +/// ⚠️ IMPORTANT: This mapping must stay in sync with `rustToolNames` in +/// `app/src/utils/toolDefinitions.ts`. When adding or modifying tools, +/// update both files. const TOOL_ID_TO_RUST_NAMES: &[(&str, &[&str])] = &[🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/tools/user_filter.rs` around lines 6 - 32, The TOOL_ID_TO_RUST_NAMES constant duplicates the rustToolNames mapping defined in app/src/utils/toolDefinitions.ts, creating a maintenance burden; add a clear comment above the TOOL_ID_TO_RUST_NAMES declaration that states this file must be updated in sync with rustToolNames in app/src/utils/toolDefinitions.ts (include that exact path and symbol name in the comment) and mention which symbol to update (TOOL_ID_TO_RUST_NAMES) so future contributors know the source of truth and where to make coordinated changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.gitmodules:
- Line 4: The submodule branch for tauri-cef was changed to "feat/cef" but the
project documentation still says it requires "feat/cef-notification-intercept";
either switch the submodule branch back to "feat/cef-notification-intercept" in
.gitmodules or confirm that "feat/cef" now contains the native Web Notifications
interception code and update the documentation (the Cargo.toml section that
mentions the required branch) to reference "feat/cef" and any changed behavior;
ensure the chosen branch actually exposes the notification-intercept
functionality before committing the doc or submodule change.
---
Outside diff comments:
In `@app/src/components/settings/panels/ToolsPanel.tsx`:
- Around line 30-41: The initialization in the useEffect compares persisted
backend names (onboardingTasks.enabledTools) against UI tool IDs (tool.id) so
multi-name tools appear disabled after save; fix by treating persisted list as
backend names: build a Set from onboardingTasks.enabledTools and for each tool
in toolsByCategory use a backend-name mapping (e.g., tool.backendNames or
tool.rustNames — whichever property holds the Rust/backend identifiers) and mark
the tool enabled if any of those backend names exist in the persisted Set;
alternatively create a reverse map from backend name -> ui id once and check
membership. Update the comparison in the useEffect (where
enabledList.includes(tool.id) is used) to check backend name membership and then
call setEnabled(map) as before.
---
Nitpick comments:
In `@app/src/components/settings/panels/ToolsPanel.tsx`:
- Around line 22-25: The current 500ms timeout guard using savingRef is brittle;
replace it by tracking a concrete save "version" or token instead: when
performing the save increment a local saveVersionRef (or generate a unique
saveToken) and set savingRef true and saveStatus accordingly, then in the
useEffect that reacts to the Redux/settings state compare the incoming
settings/version to that saveVersion/token and only clear savingRef and
setSaveStatus('saved') when the effect observes the new version/token; reference
and update savingRef, saveStatus/setSaveStatus and the existing useEffect that
currently resets the guard so the guard is cleared deterministically when the
new data actually arrives instead of after a fixed 500ms timeout.
In `@src/openhuman/tools/user_filter.rs`:
- Around line 6-32: The TOOL_ID_TO_RUST_NAMES constant duplicates the
rustToolNames mapping defined in app/src/utils/toolDefinitions.ts, creating a
maintenance burden; add a clear comment above the TOOL_ID_TO_RUST_NAMES
declaration that states this file must be updated in sync with rustToolNames in
app/src/utils/toolDefinitions.ts (include that exact path and symbol name in the
comment) and mention which symbol to update (TOOL_ID_TO_RUST_NAMES) so future
contributors know the source of truth and where to make coordinated changes.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 125147c4-6bea-4d88-8326-14415d03adde
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockapp/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
.claude/memory.md.gitmodulesapp/src/components/settings/panels/ToolsPanel.tsxapp/src/utils/toolDefinitions.tssrc/openhuman/agent/harness/session/builder.rssrc/openhuman/app_state/ops.rssrc/openhuman/tools/mod.rssrc/openhuman/tools/user_filter.rs
| path = app/src-tauri/vendor/tauri-cef | ||
| url = https://github.com/tinyhumansai/tauri-cef.git | ||
| branch = feat/cef-notification-intercept | ||
| branch = feat/cef |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== .gitmodules branch setting =="
git config -f .gitmodules --get submodule.app/src-tauri/vendor/tauri-cef.branch || true
echo
echo "== Cargo.toml notes referencing expected branch/intercept API =="
rg -n -C3 'feat/cef-notification-intercept|tauri-runtime-cef::notification' app/src-tauri/Cargo.toml || true
echo
echo "== Remote branches in tinyhumansai/tauri-cef =="
git ls-remote --heads https://github.com/tinyhumansai/tauri-cef.git 'feat/cef*' || true
echo
echo "== If submodule is present locally, show pinned commit =="
if [ -d app/src-tauri/vendor/tauri-cef/.git ]; then
git -C app/src-tauri/vendor/tauri-cef rev-parse HEAD
else
echo "Submodule working tree not present in this checkout."
fiRepository: tinyhumansai/openhuman
Length of output: 1571
Submodule branch changed without updating documentation: notification features may be missing.
The .gitmodules branch for app/src-tauri/vendor/tauri-cef is now set to feat/cef, but app/src-tauri/Cargo.toml:92-98 explicitly documents the project requires feat/cef-notification-intercept because it includes native Web Notifications interception support. Update either the submodule branch back to feat/cef-notification-intercept or verify that feat/cef now includes the notification interception features and update the documentation accordingly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.gitmodules at line 4, The submodule branch for tauri-cef was changed to
"feat/cef" but the project documentation still says it requires
"feat/cef-notification-intercept"; either switch the submodule branch back to
"feat/cef-notification-intercept" in .gitmodules or confirm that "feat/cef" now
contains the native Web Notifications interception code and update the
documentation (the Cargo.toml section that mentions the required branch) to
reference "feat/cef" and any changed behavior; ensure the chosen branch actually
exposes the notification-intercept functionality before committing the doc or
submodule change.
…t runtime (tinyhumansai#720) * chore: update OpenHuman version to 0.52.26 and refine tool preference handling - Bumped the OpenHuman package version to 0.52.26 in Cargo.lock files. - Enhanced the ToolsPanel component to include user feedback on save status. - Implemented filtering of tools based on user preferences in the Rust backend, allowing for more customizable tool availability. - Added new utility functions to map UI tool IDs to Rust tool names for better integration. These changes improve user experience and maintain compatibility with the latest features. * style: apply cargo fmt to user_filter.rs
Summary
ToolsPanelnow stores the expanded set of Rust tool names (viagetEnabledRustToolNames) so the backend can directly filter without re-mappingenabled_toolsand removes disabled tools before building the session — tools not in the UI catalog (agent infrastructure tools) are always retainedsavingRefprevents the effect from re-initialising toggle state immediately after a save triggers a Redux refreshtoolDefinitions.tsnow carriesrustToolNamesper tool;user_filter.rsholds the canonical Rust-side mapping (including theweb_search→web_search_toolandcron→ 6 tool names corrections)Files changed
app/src/utils/toolDefinitions.tsrustToolNamesfield +getEnabledRustToolNames()exportapp/src/components/settings/panels/ToolsPanel.tsxsrc/openhuman/tools/user_filter.rsfilter_tools_by_user_preference()src/openhuman/tools/mod.rsuser_filtermodulesrc/openhuman/agent/harness/session/builder.rssrc/openhuman/app_state/ops.rsload_stored_app_statemadepub(crate)Checklist
yarn typecheck)yarn lint)yarn format:check)cargo check)enabled_toolslist = all tools enabled (safe default)Closes #445
Summary by CodeRabbit
New Features
Documentation
Chores