refactor: remove hardware-related components and streamline service management#502
refactor: remove hardware-related components and streamline service management#502senamakel merged 5 commits intotinyhumansai:mainfrom
Conversation
- Added support for a pre-login user directory to encapsulate configuration, memory, and state before any user logs in. This ensures that all initial data is scoped under a dedicated user directory (`users/local`), preventing direct writes to the root `.openhuman` path. - Implemented the `pre_login_user_dir` function to return the appropriate path for the pre-login user. - Updated configuration loading logic to defer disk state creation until the first successful login, enhancing user data management and isolation. - Added tests to verify the correct behavior of the pre-login directory structure.
…anagement - Deleted hardware configuration and related tools from the codebase, including `HardwareConfig`, `HardwareTransport`, and associated memory management tools. - Introduced a new `service.ts` module for managing service and daemon commands, consolidating service-related functionalities. - Updated import paths across the application to reflect the removal of hardware references and the addition of the new service management module. - Refactored the `build_system_prompt` function to remove hardware access instructions, focusing on action instructions instead. - Cleaned up the Cargo.toml and Cargo.lock files by removing unused dependencies related to hardware management.
📝 WalkthroughWalkthroughRemoves hardware-related crates and Cargo features, deletes hardware config schema and hardware tools, updates runtime config resolution to add a pre-login per-user directory, adjusts Tauri service export/imports, and removes hardware-specific system-prompt text. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/openhuman/channels/prompt.rs (1)
74-80:⚠️ Potential issue | 🟠 MajorGuard tool-usage instruction behind tool availability
Line 77 currently mandates tool usage even when
toolsis empty, which can push the model to emit invalid<tool_call>tags. Make this instruction conditional so no-tool channels get a direct-response directive instead.Proposed fix
- // ── 1b. Action instruction (avoid meta-summary) ─────────────── - prompt.push_str( - "## Your Task\n\n\ - When the user sends a message, ACT on it. Use the tools to fulfill their request.\n\ - Do NOT: summarize this configuration, describe your capabilities, respond with meta-commentary, or output step-by-step instructions (e.g. \"1. First... 2. Next...\").\n\ - Instead: emit actual <tool_call> tags when you need to act. Just do what they ask.\n\n", - ); + // ── 1b. Action instruction (avoid meta-summary) ─────────────── + if tools.is_empty() { + prompt.push_str( + "## Your Task\n\n\ + When the user sends a message, ACT on it directly.\n\ + Do NOT: summarize this configuration, describe your capabilities, or respond with meta-commentary.\n\ + There are no tools available in this channel, so respond normally without <tool_call> tags.\n\n", + ); + } else { + prompt.push_str( + "## Your Task\n\n\ + When the user sends a message, ACT on it. Use the tools to fulfill their request.\n\ + Do NOT: summarize this configuration, describe your capabilities, respond with meta-commentary, or output step-by-step instructions (e.g. \"1. First... 2. Next...\").\n\ + Instead: emit actual <tool_call> tags when you need to act. Just do what they ask.\n\n", + ); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/channels/prompt.rs` around lines 74 - 80, The "## Your Task" block currently forces tool usage even when no tools are present; modify the code around prompt.push_str so it checks the channel's tools list (e.g., tools or whatever variable holds tool availability) and only injects the "emit <tool_call> tags" instruction when tools is non-empty, otherwise push an alternative directive telling the model to produce a direct response (no tool tags, no meta-commentary). Ensure you update the logic that builds the prompt (referencing prompt.push_str and the "## Your Task" string) so the two different strings are chosen based on tools.is_empty() / tools.len() == 0.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/openhuman/channels/prompt.rs`:
- Around line 74-80: The "## Your Task" block currently forces tool usage even
when no tools are present; modify the code around prompt.push_str so it checks
the channel's tools list (e.g., tools or whatever variable holds tool
availability) and only injects the "emit <tool_call> tags" instruction when
tools is non-empty, otherwise push an alternative directive telling the model to
produce a direct response (no tool tags, no meta-commentary). Ensure you update
the logic that builds the prompt (referencing prompt.push_str and the "## Your
Task" string) so the two different strings are chosen based on tools.is_empty()
/ tools.len() == 0.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cf5f8233-e704-4932-8765-2601a185e420
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockapp/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
Cargo.tomlapp/src/lib/coreState/store.tsapp/src/services/coreStateApi.tsapp/src/utils/tauriCommands/index.tsapp/src/utils/tauriCommands/service.tssrc/openhuman/channels/prompt.rssrc/openhuman/config/mod.rssrc/openhuman/config/schema/hardware.rssrc/openhuman/config/schema/load.rssrc/openhuman/config/schema/mod.rssrc/openhuman/config/schema/types.rssrc/openhuman/tools/impl/hardware/board_info.rssrc/openhuman/tools/impl/hardware/memory_map.rssrc/openhuman/tools/impl/hardware/memory_read.rssrc/openhuman/tools/impl/hardware/mod.rssrc/openhuman/tools/impl/mod.rs
💤 Files with no reviewable changes (8)
- src/openhuman/tools/impl/mod.rs
- src/openhuman/config/schema/types.rs
- src/openhuman/tools/impl/hardware/memory_read.rs
- Cargo.toml
- src/openhuman/config/schema/hardware.rs
- src/openhuman/tools/impl/hardware/mod.rs
- src/openhuman/tools/impl/hardware/memory_map.rs
- src/openhuman/tools/impl/hardware/board_info.rs
…nction - Introduced a `write_config_file` function to encapsulate the logic for creating directories and writing configuration files, improving code reuse and readability. - Updated test cases to utilize the new function for writing configuration files, ensuring consistency and reducing duplication. - Added handling for pre-login user directory structure to ensure configuration is correctly written to the appropriate paths.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/json_rpc_e2e.rs (1)
545-549: Consolidate duplicated config-writer helper to prevent drift
write_config_fileis duplicated in both helpers (Line 545 and Line 581). Pulling it to a single file-level helper avoids future divergence when write semantics change.[Line 545] and [Line 581] currently define the same logic independently.
♻️ Suggested refactor
+fn write_config_file(config_dir: &Path, cfg: &str) { + std::fs::create_dir_all(config_dir).expect("mkdir openhuman"); + let path = config_dir.join("config.toml"); + std::fs::write(&path, cfg).expect("write config"); +} + fn write_min_config(openhuman_dir: &Path, api_origin: &str) { @@ - fn write_config_file(config_dir: &Path, cfg: &str) { - std::fs::create_dir_all(config_dir).expect("mkdir openhuman"); - let path = config_dir.join("config.toml"); - std::fs::write(&path, cfg).expect("write config"); - } - write_config_file(openhuman_dir, &cfg); @@ fn write_min_config_with_local_ai_disabled(openhuman_dir: &Path, api_origin: &str) { @@ - fn write_config_file(config_dir: &Path, cfg: &str) { - std::fs::create_dir_all(config_dir).expect("mkdir openhuman"); - let path = config_dir.join("config.toml"); - std::fs::write(&path, cfg).expect("write config"); - } - write_config_file(openhuman_dir, &cfg);Also applies to: 581-585
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/json_rpc_e2e.rs` around lines 545 - 549, Two identical helpers named write_config_file are duplicated; remove the duplicate and consolidate to a single file-level helper. Create one top-level function write_config_file(config_dir: &Path, cfg: &str) that contains the mkdir and write logic, delete the second definition, and update any local test callers to use that single helper; ensure the function signature/visibility matches the callers so no additional changes are required.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/json_rpc_e2e.rs`:
- Around line 545-549: Two identical helpers named write_config_file are
duplicated; remove the duplicate and consolidate to a single file-level helper.
Create one top-level function write_config_file(config_dir: &Path, cfg: &str)
that contains the mkdir and write logic, delete the second definition, and
update any local test callers to use that single helper; ensure the function
signature/visibility matches the callers so no additional changes are required.
Summary
hardwaretoserviceto match the streamlined service-management surface.Problem
mainwith the current branch contents pushed and a clear summary of the mixed cleanup/config work.Solution
fix/fridayand open a PR fromsenamakel:fix/fridaytotinyhumansai:main.Submission Checklist
Impact
Related
react-hooks/set-state-in-effectwarnings that still appear during the push hook.Summary by CodeRabbit
Refactor
New Features
Tests