Skip to content

chore(dev): add Windows dev runner and align lockfile versions#1015

Merged
senamakel merged 8 commits into
tinyhumansai:mainfrom
M3gA-Mind:feat/windows-leak-check
Apr 29, 2026
Merged

chore(dev): add Windows dev runner and align lockfile versions#1015
senamakel merged 8 commits into
tinyhumansai:mainfrom
M3gA-Mind:feat/windows-leak-check

Conversation

@M3gA-Mind
Copy link
Copy Markdown
Contributor

@M3gA-Mind M3gA-Mind commented Apr 29, 2026

Add a Windows-focused dev startup script that auto-discovers pnpm/ninja and configures CEF tooling, while also bumping lockfile package versions to 0.53.4 for consistency.

Made-with: Cursor

Summary

  • What changed and why.
  • Keep this to 3-6 bullets focused on user-visible or architecture-impacting changes.

Problem

  • What issue or risk this PR addresses.
  • Include context needed for reviewers to evaluate correctness quickly.

Solution

  • How the implementation solves the problem.
  • Note important design decisions and tradeoffs.

Submission Checklist

  • Unit tests — Vitest (app/) and/or cargo test (core) for logic you add or change
  • E2E / integration — Where behavior is user-visible or crosses UI → Tauri → sidecar → JSON-RPC; use existing harnesses (app/test/e2e, mock backend, tests/json_rpc_e2e.rs as appropriate)
  • N/A — If truly not applicable, say why (e.g. change is documentation-only)
  • Doc comments/// / //! (Rust), JSDoc or brief file/module headers (TS) on public APIs and non-obvious modules
  • Inline comments — Where logic, invariants, or edge cases aren’t clear from names alone (keep them grep-friendly; avoid restating the code)

(Any feature related checklist can go in here)

Impact

  • Runtime/platform impact (desktop/mobile/web/CLI), if any.
  • Performance, security, migration, or compatibility implications.

Related

  • Closes:
  • Follow-up PR(s)/TODOs:

Summary by CodeRabbit

  • New Features

    • Added a Windows/Git-Bash dev runner for local development workflows.
  • Improvements

    • Screen Intelligence panel now shows the permissions section only on supported platforms and clarifies macOS-only messaging.
  • Platform Support

    • Better Windows handling for dev tooling and pre-push checks to reduce setup friction.
  • Tests

    • Multiple tests made cross-platform resilient to path and behavior differences.
  • Chores

    • Ignore and formatting lists updated to skip a temporary test-target directory.

Add a Windows-focused dev startup script that auto-discovers pnpm/ninja and configures CEF tooling, while also bumping lockfile package versions to 0.53.4 for consistency.

Made-with: Cursor
@M3gA-Mind M3gA-Mind requested a review from a team April 29, 2026 08:36
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 29, 2026

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f6b76cad-817a-483c-8c64-70b24e30f371

📥 Commits

Reviewing files that changed from the base of the PR and between d9e28a2 and d8f97a5.

📒 Files selected for processing (2)
  • scripts/run-dev-win.sh
  • src/openhuman/composio/periodic.rs

📝 Walkthrough

Walkthrough

Adds Windows-focused developer tooling and CI script support, broadens platform-aware test behavior across the codebase, conditions UI permission controls on platform support, and updates ignore/config files for Windows dev flows.

Changes

Cohort / File(s) Summary
Windows Dev Runner & Hooks
scripts/run-dev-win.sh, .husky/pre-push
Adds a new Git Bash/Windows dev runner that resolves pnpm, ninja, CMake, and CEF locations and runs Tauri pnpm tasks; enhances pre-push hook with Windows-specific node/pnpm detection, PATH recovery via where.exe, and early failure if tools are missing.
Ignore / Formatting
.gitignore, app/.prettierignore
Adds target-test-run to ignore lists; fixes trailing newline in .gitignore.
Settings UI & Tests
app/src/components/settings/panels/ScreenIntelligencePanel.tsx, app/src/components/settings/panels/__tests__/ScreenIntelligencePanel.test.tsx
Render PermissionsSection only when platform_supported is true; clarify macOS-only messaging; update tests to assert absence/presence accordingly.
Cross-platform Test Path Handling
src/openhuman/local_ai/paths.rs, src/openhuman/tools/impl/system/node_exec.rs, src/openhuman/tools/impl/system/npm_exec.rs, src/openhuman/tools/impl/system/shell.rs, src/openhuman/tools/impl/network/curl.rs
Tests switched to platform-aware absolute-path samples or normalized separators; shell tests select platform-appropriate commands and are gated from running on Windows where needed.
Platform-Gated Tests & Timeouts
src/openhuman/cron/scheduler_tests.rs, src/openhuman/composio/periodic.rs, src/openhuman/tools/impl/browser/screenshot.rs
Added #[cfg(not(windows))] to Unix-only tests; enforce a 5s timeout on an async tick test; screenshot tests avoid platform-specific command assumptions.
Windows Locking & History Behavior
src/openhuman/composio/trigger_history.rs, src/openhuman/tools/impl/cron/run.rs
Switch to process-local mutex for archive writes on Windows instead of file locks; cron run test branches on Windows expecting spawn error and adjusts persisted-history assertions.
Security/Policy & Skill Tests
src/openhuman/security/policy_tests.rs, src/openhuman/skills/ops_tests.rs
Make path-blocking and absolute-path rejection tests platform-aware (Windows vs Unix absolute paths).
Agent / Tooling Test Adjustments
src/openhuman/agent/harness/self_healing.rs, src/openhuman/tools/impl/browser/screenshot.rs, src/openhuman/tools/impl/network/curl.rs
Normalize path separators in tests; adjust unsafe-character expectations and platform-conditional assertions.
E2E Mutex & Cleanup Handling
tests/autocomplete_memory_e2e.rs
Recover from poisoned mutex guards, run history cleanup at test start and treat cleanup failures non-fatally.
Misc. Test-only Changes
various src/... test modules
Multiple test updates to use platform-appropriate paths, gating, or normalization to ensure cross-platform reliability.

Sequence Diagram(s)

sequenceDiagram
    participant DevShell as Developer Shell
    participant Runner as scripts/run-dev-win.sh
    participant FS as Filesystem / WinGet dirs
    participant Tools as pnpm/ninja/CMake
    participant CEF as CEF runtime
    participant Tauri as pnpm (Tauri tasks)

    DevShell->>Runner: invoke run-dev-win.sh
    Runner->>FS: compute repo/app/script paths\nsource load-dotenv.sh
    Runner->>FS: locate WinGet package dirs\n(find newest pnpm/ninja)
    FS-->>Runner: return tool paths
    Runner->>Tools: export CMAKE_MAKE_PROGRAM / update PATH
    Runner->>FS: locate CEF runtime under LOCALAPPDATA
    FS-->>Runner: return CEF runtime (if found)
    Runner->>Runner: update PATH with CEF runtime
    Runner->>Tauri: run `pnpm --filter ... tauri:ensure`, `core:stage`, `tauri dev`
    Tauri-->>DevShell: start dev server / Tauri dev output
Loading

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly Related PRs

Suggested Reviewers

  • senamakel
  • graycyrus

Poem

🐰 I found a path with many slashes,

backslashes turned into tiny dashes,
Hooks now hop to Windows tune,
Tests behave on sun and moon,
Hooray — dev runs bloom like clover! 🥕

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main changes: adding a Windows dev runner script and aligning versions, which are the primary focus of the changeset across multiple files.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Review rate limit: 4/5 reviews remaining, refill in 12 minutes.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
scripts/run-dev-win.sh (1)

19-49: Consider extracting a shared WinGet executable resolver.

find_pnpm and find_ninja duplicate the same control flow; one helper would reduce drift and simplify future updates.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/run-dev-win.sh` around lines 19 - 49, find_pnpm and find_ninja
duplicate the same WinGet lookup/control flow; extract a single helper (e.g.,
find_winget_exe or resolve_winget_package) that accepts the package pattern/name
and expected exe filename, performs the to_unix_path("${LOCALAPPDATA:-}")
conversion, finds the package directory with
ls/.../Microsoft/WinGet/Packages/<pattern>_* , checks for -x
"<candidate>/<exe>.exe" and prints the path if found, and returns appropriate
status; then simplify find_pnpm and find_ninja to call that helper (keeping
existing behavior of checking command -v first and falling back to the helper)
so both functions reuse the shared resolver.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@scripts/run-dev-win.sh`:
- Around line 12-17: The to_unix_path function should fail fast when cygpath is
not installed; modify to_unix_path to first check for cygpath availability
(e.g., command -v cygpath or type -P cygpath) and if missing print a clear error
to stderr and exit with a non-zero status so callers don’t proceed and produce
misleading “pnpm/ninja not found” errors; update every instance/definition of
to_unix_path (the function used in both the early block and the later block
around lines 51-61) to perform this check before attempting cygpath -u and
validate the input argument as currently done.
- Line 27: The script currently picks the first WinGet package directory match
into the candidate variable using head -n1 which can return an older install;
change the selection logic in the candidate assignment (and the identical
occurrence later) to pick the latest package directory instead — e.g., list
matches, sort them by version (or by name) and choose the last entry so
candidate points to the newest pnpm.pnpm_* directory; update both places that
use head -n1 (the candidate assignment and the similar line around Line 43) to
use this “latest” selection approach.

---

Nitpick comments:
In `@scripts/run-dev-win.sh`:
- Around line 19-49: find_pnpm and find_ninja duplicate the same WinGet
lookup/control flow; extract a single helper (e.g., find_winget_exe or
resolve_winget_package) that accepts the package pattern/name and expected exe
filename, performs the to_unix_path("${LOCALAPPDATA:-}") conversion, finds the
package directory with ls/.../Microsoft/WinGet/Packages/<pattern>_* , checks for
-x "<candidate>/<exe>.exe" and prints the path if found, and returns appropriate
status; then simplify find_pnpm and find_ninja to call that helper (keeping
existing behavior of checking command -v first and falling back to the helper)
so both functions reuse the shared resolver.
🪄 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: 2cb33e59-fcf4-4fa6-aba7-2a1b6cc033dc

📥 Commits

Reviewing files that changed from the base of the PR and between b11b8f3 and 27d5d6c.

⛔ Files ignored due to path filters (2)
  • Cargo.lock is excluded by !**/*.lock
  • app/src-tauri/Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (1)
  • scripts/run-dev-win.sh

Comment thread scripts/run-dev-win.sh
Comment thread scripts/run-dev-win.sh Outdated
Adjust settings messaging and harden Rust test suites for Windows path/runtime differences so staged frontend and core tests behave consistently across environments.

Made-with: Cursor
Refactor test assertions across multiple files for better readability by aligning code formatting and ensuring consistent style. This includes adjustments in the ScreenIntelligencePanel test, Rust tests for skill resources, and various shell command tests.

Made-with: Cursor
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
src/openhuman/composio/trigger_history.rs (1)

116-141: ⚠️ Potential issue | 🟠 Major

Restore write synchronization on Windows to avoid archive corruption

Gating out locking on Windows removes write serialization in record_trigger; concurrent appends can race and produce malformed/incomplete JSONL lines. Keep a Windows-safe lock path (at least process-local mutex) instead of fully unsynchronized writes.

Suggested patch (Windows mutex fallback)
 use std::sync::{Arc, OnceLock};
+#[cfg(windows)]
+use std::sync::Mutex;
 
 static GLOBAL_TRIGGER_HISTORY: OnceLock<Arc<ComposioTriggerHistoryStore>> = OnceLock::new();
+#[cfg(windows)]
+static WINDOWS_TRIGGER_WRITE_LOCK: OnceLock<Mutex<()>> = OnceLock::new();
@@
-        #[cfg(not(windows))]
+        #[cfg(windows)]
+        let _windows_guard = WINDOWS_TRIGGER_WRITE_LOCK
+            .get_or_init(|| Mutex::new(()))
+            .lock()
+            .map_err(|_| {
+                format!(
+                    "[composio][history] windows write mutex poisoned for archive file {}",
+                    path.display()
+                )
+            })?;
+
+        #[cfg(not(windows))]
         file.lock_exclusive().map_err(|error| {
             format!(
                 "[composio][history] failed to lock archive file {}: {error}",
                 path.display()
             )
         })?;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/composio/trigger_history.rs` around lines 116 - 141, The
Windows build currently skips file locking in record_trigger, allowing
concurrent appends to corrupt the JSONL archive; restore synchronization by
using a Windows-safe fallback (a process-local mutex) instead of removing
locking entirely: in trigger_history::record_trigger acquire a global/static
Mutex (e.g., a Lazy/static Mutex<()>) on Windows before performing writeln! and
flush, map mutex lock errors into the same formatted error strings as the
existing file.lock_exclusive/unlock branches, and release the mutex after flush;
keep the existing file.lock_exclusive()/unlock() behavior on non-Windows targets
and ensure write_result? and unlock/fallback-mutex release happen after
successful write/flush.
🧹 Nitpick comments (3)
src/openhuman/tools/impl/browser/screenshot.rs (1)

357-359: Prefer partial platform adaptation over full test skip.

Line 357 skips the whole unsafe-filename test on unsupported OS, which drops coverage for the shell-safety guard. Consider keeping the test active and only making path-separator-sensitive cases platform-conditional.

♻️ Suggested adjustment
-        if !matches!(std::env::consts::OS, "macos" | "linux") {
-            return;
-        }
         let tool = ScreenshotTool::new(test_security());
-        for ch in ['\'', '"', '`', '$', '\\', ';', '|', '&', '(', ')'] {
+        let mut chars = vec!['\'', '"', '`', '$', ';', '|', '&', '(', ')'];
+        if matches!(std::env::consts::OS, "macos" | "linux") {
+            chars.push('\\');
+        }
+        for ch in chars {
             let filename = format!("test{ch}name.png");
             let result = tool
                 .execute(serde_json::json!({"filename": filename}))
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/tools/impl/browser/screenshot.rs` around lines 357 - 359, The
test currently returns early using the if !matches!(std::env::consts::OS,
"macos" | "linux") { return; } guard which skips the entire unsafe-filename test
on non-mac/linux; instead remove the full-test early return and keep the test
active, making only the path-separator or platform-specific assertions
conditional (use cfg!(target_os = "...") or check std::path::MAIN_SEPARATOR to
branch expectations), or normalize paths (Path/PathBuf and to_string_lossy) so
assertions that are platform-agnostic still run; update the code around the
std::env::consts::OS check to only wrap/adjust the specific path-sensitive cases
rather than skipping the whole test.
tests/autocomplete_memory_e2e.rs (1)

57-57: Consider making the best-effort cleanup explicit.

Silently discarding clear_history() errors can make setup regressions harder to diagnose. A brief comment/log on Err would keep intent clear without making tests brittle.

♻️ Minimal clarity tweak
-    let _ = history::clear_history().await;
+    if let Err(_e) = history::clear_history().await {
+        // Best-effort cleanup: each test has isolated HOME/temp workspace.
+    }

Also applies to: 107-107, 158-158, 200-200

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/autocomplete_memory_e2e.rs` at line 57, The call to
history::clear_history().await is currently discarding errors (let _ = ...);
change it to explicitly handle the Result by matching or using if let Err(e) =
history::clear_history().await { /* log or comment */ } so failures are noted;
include a short log message or test comment that this is a best-effort cleanup
(do the same for the other occurrences) to make setup regressions visible
without failing the test.
src/openhuman/tools/impl/system/shell.rs (1)

347-347: Use camelCase for newly introduced local variables.

At Line 347 and Line 359, home_cmd and path_cmd don’t follow the repository’s Rust naming rule.

Proposed fix
-        let home_cmd = if cfg!(windows) {
+        let homeCmd = if cfg!(windows) {
             "echo $env:USERPROFILE"
         } else {
             "echo $HOME"
         };
-        let result = tool.execute(json!({"command": home_cmd})).await.unwrap();
+        let result = tool.execute(json!({"command": homeCmd})).await.unwrap();
@@
-        let path_cmd = if cfg!(windows) {
+        let pathCmd = if cfg!(windows) {
             "echo $env:PATH"
         } else {
             "echo $PATH"
         };
-        let result = tool.execute(json!({"command": path_cmd})).await.unwrap();
+        let result = tool.execute(json!({"command": pathCmd})).await.unwrap();

As per coding guidelines src/openhuman/**/*.rs: “Use camelCase for variable names”.

Also applies to: 359-359

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/tools/impl/system/shell.rs` at line 347, Rename the newly
introduced locals home_cmd and path_cmd to camelCase (e.g., homeCmd and pathCmd)
and update all usages and pattern matches accordingly in the same module; locate
these identifiers in the shell command construction code (the block that sets
let home_cmd = if cfg!(windows) { ... } and the similar let path_cmd = ...) and
replace every reference so compilation and name resolution remain correct, then
run cargo build/tests to ensure no remaining references or clippy warnings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/openhuman/tools/impl/cron/run.rs`:
- Around line 117-125: The test currently returns early inside the cfg!(windows)
branch which skips the later history assertions; modify the Windows-specific
branch so it still checks the Windows-specific expectations (assert! on
result.is_error and spawn error in result.output()) but do not return — allow
execution to continue to call cron::list_runs and assert the records_history
behavior. Specifically, remove the early `return` in the Windows branch (or move
the Windows-only asserts into a block that does not exit), keeping references to
`result`, `result.output()`, and `cron::list_runs` and ensuring the final
`records_history` assertion runs on Windows as well.

---

Outside diff comments:
In `@src/openhuman/composio/trigger_history.rs`:
- Around line 116-141: The Windows build currently skips file locking in
record_trigger, allowing concurrent appends to corrupt the JSONL archive;
restore synchronization by using a Windows-safe fallback (a process-local mutex)
instead of removing locking entirely: in trigger_history::record_trigger acquire
a global/static Mutex (e.g., a Lazy/static Mutex<()>) on Windows before
performing writeln! and flush, map mutex lock errors into the same formatted
error strings as the existing file.lock_exclusive/unlock branches, and release
the mutex after flush; keep the existing file.lock_exclusive()/unlock() behavior
on non-Windows targets and ensure write_result? and unlock/fallback-mutex
release happen after successful write/flush.

---

Nitpick comments:
In `@src/openhuman/tools/impl/browser/screenshot.rs`:
- Around line 357-359: The test currently returns early using the if
!matches!(std::env::consts::OS, "macos" | "linux") { return; } guard which skips
the entire unsafe-filename test on non-mac/linux; instead remove the full-test
early return and keep the test active, making only the path-separator or
platform-specific assertions conditional (use cfg!(target_os = "...") or check
std::path::MAIN_SEPARATOR to branch expectations), or normalize paths
(Path/PathBuf and to_string_lossy) so assertions that are platform-agnostic
still run; update the code around the std::env::consts::OS check to only
wrap/adjust the specific path-sensitive cases rather than skipping the whole
test.

In `@src/openhuman/tools/impl/system/shell.rs`:
- Line 347: Rename the newly introduced locals home_cmd and path_cmd to
camelCase (e.g., homeCmd and pathCmd) and update all usages and pattern matches
accordingly in the same module; locate these identifiers in the shell command
construction code (the block that sets let home_cmd = if cfg!(windows) { ... }
and the similar let path_cmd = ...) and replace every reference so compilation
and name resolution remain correct, then run cargo build/tests to ensure no
remaining references or clippy warnings.

In `@tests/autocomplete_memory_e2e.rs`:
- Line 57: The call to history::clear_history().await is currently discarding
errors (let _ = ...); change it to explicitly handle the Result by matching or
using if let Err(e) = history::clear_history().await { /* log or comment */ } so
failures are noted; include a short log message or test comment that this is a
best-effort cleanup (do the same for the other occurrences) to make setup
regressions visible without failing the test.
🪄 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: cbe25caf-e4e8-43e9-831d-22033c688449

📥 Commits

Reviewing files that changed from the base of the PR and between 27d5d6c and 8eec068.

📒 Files selected for processing (17)
  • .gitignore
  • app/src/components/settings/panels/ScreenIntelligencePanel.tsx
  • app/src/components/settings/panels/__tests__/ScreenIntelligencePanel.test.tsx
  • src/openhuman/agent/harness/self_healing.rs
  • src/openhuman/composio/periodic.rs
  • src/openhuman/composio/trigger_history.rs
  • src/openhuman/cron/scheduler_tests.rs
  • src/openhuman/local_ai/paths.rs
  • src/openhuman/security/policy_tests.rs
  • src/openhuman/skills/ops_tests.rs
  • src/openhuman/tools/impl/browser/screenshot.rs
  • src/openhuman/tools/impl/cron/run.rs
  • src/openhuman/tools/impl/network/curl.rs
  • src/openhuman/tools/impl/system/node_exec.rs
  • src/openhuman/tools/impl/system/npm_exec.rs
  • src/openhuman/tools/impl/system/shell.rs
  • tests/autocomplete_memory_e2e.rs
✅ Files skipped from review due to trivial changes (4)
  • src/openhuman/skills/ops_tests.rs
  • src/openhuman/cron/scheduler_tests.rs
  • src/openhuman/tools/impl/system/node_exec.rs
  • .gitignore

Comment thread src/openhuman/tools/impl/cron/run.rs
Add 'target-test-run' to .prettierignore and correct the syntax in the lint:commands-tokens script for improved compatibility.
Harden husky pre-push to auto-discover node and pnpm via where.exe/cygpath in Git Bash so pushes don't fail when PATH is incomplete.

Made-with: Cursor
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.husky/pre-push:
- Around line 33-42: The hook attempts to recover pnpm via
prepend_windows_exe_dir but does not fail when pnpm remains absent, causing
downstream noisy failures; after the recovery attempt in the pre-push script
check for pnpm (e.g., using command -v pnpm or the existing pnpm check) and if
still missing produce a clear error message and exit non-zero immediately
(similar to the existing has_node guard), so add a hard guard that echoes an
actionable message about installing/exposing pnpm and calls exit 1 before
proceeding to run Prettier/ESLint/tsc/Rust checks.
🪄 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: eaf851a6-7f38-4548-8b7f-415a3e54c241

📥 Commits

Reviewing files that changed from the base of the PR and between 8eec068 and a6bf91d.

📒 Files selected for processing (3)
  • .husky/pre-push
  • app/.prettierignore
  • app/package.json
✅ Files skipped from review due to trivial changes (2)
  • app/.prettierignore
  • app/package.json

Comment thread .husky/pre-push
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 29, 2026

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details
{"name":"HttpError","status":401,"request":{"method":"PATCH","url":"https://api.github.com/repos/tinyhumansai/openhuman/issues/comments/4342093632","headers":{"accept":"application/vnd.github.v3+json","user-agent":"octokit.js/0.0.0-development octokit-core.js/7.0.6 Node.js/24","authorization":"token [REDACTED]","content-type":"application/json; charset=utf-8"},"body":{"body":"<!-- This is an auto-generated comment: summarize by coderabbit.ai -->\nNo actionable comments were generated in the recent review. 🎉\n\n<details>\n<summary>ℹ️ Recent review info</summary>\n\n<details>\n<summary>⚙️ Run configuration</summary>\n\n**Configuration used**: defaults\n\n**Review profile**: CHILL\n\n**Plan**: Pro\n\n**Run ID**: `963739a2-0d27-4a30-9d57-1990e7af433b`\n\n</details>\n\n<details>\n<summary>📥 Commits</summary>\n\nReviewing files that changed from the base of the PR and between a6bf91d4acc7d886e36633cfb4ea3088be8da6e2 and 2b95f662521be754cbd7b918c8666cd225e9c0d0.\n\n</details>\n\n<details>\n<summary>📒 Files selected for processing (2)</summary>\n\n* `.gitignore`\n* `src/openhuman/cron/scheduler_tests.rs`\n\n</details>\n\n<details>\n<summary>✅ Files skipped from review due to trivial changes (2)</summary>\n\n* .gitignore\n* src/openhuman/cron/scheduler_tests.rs\n\n</details>\n\n</details>\n\n---\n\n\n<!-- walkthrough_start -->\n\n<details>\n<summary>📝 Walkthrough</summary>\n\n## Walkthrough\n\nThis PR introduces Windows development tooling support via a new build script, adds comprehensive platform-aware test fixes throughout the Rust codebase to handle Windows path separators and conditional execution, refines the ScreenIntelligencePanel UI to conditionally display permissions based on platform support, and enhances the pre-push hook with Windows PATH recovery mechanisms.\n\n## Changes\n\n| Cohort / File(s) | Summary |\n|---|---|\n| **Windows Development Environment** <br> `scripts/run-dev-win.sh`, `.husky/pre-push` | New Windows dev runner script with pnpm/ninja locators and environment setup; pre-push hook now recovers Windows PATH for node/pnpm before running checks. |\n| **Ignore Configuration Updates** <br> `.gitignore`, `app/.prettierignore` | Added `target-test-run` to ignore patterns in both files. |\n| **React Component Platform Awareness** <br> `app/src/components/settings/panels/ScreenIntelligencePanel.tsx`, `app/src/components/settings/panels/__tests__/ScreenIntelligencePanel.test.tsx` | PermissionsSection now conditionally renders based on `platform_supported` status; test updated to verify section absence and macOS-specific messaging. |\n| **Rust Test Path Normalization** <br> `src/openhuman/agent/harness/self_healing.rs`, `src/openhuman/tools/impl/browser/screenshot.rs`, `src/openhuman/tools/impl/network/curl.rs`, `src/openhuman/tools/impl/system/node_exec.rs`, `src/openhuman/tools/impl/system/npm_exec.rs` | Tests normalize Windows backslash separators to forward slashes or use `cfg!(windows)` conditionals for platform-appropriate absolute path assertions. |\n| **Rust Conditional Compilation** <br> `src/openhuman/composio/trigger_history.rs`, `src/openhuman/cron/scheduler_tests.rs`, `src/openhuman/tools/impl/system/shell.rs` | Added `#[cfg(not(windows))]` guards to filesystem locks and Unix-specific scheduler/shell tests; shell tests updated with platform-dependent command variants. |\n| **Rust Platform-Specific Tests** <br> `src/openhuman/composio/periodic.rs`, `src/openhuman/local_ai/paths.rs`, `src/openhuman/security/policy_tests.rs`, `src/openhuman/skills/ops_tests.rs`, `src/openhuman/tools/impl/cron/run.rs` | Tests made Windows-aware: timeout enforcement, absolute path assertions using `cfg!(windows)` branching, cron execution failure expectations on Windows. |\n| **E2E Test Robustness** <br> `tests/autocomplete_memory_e2e.rs` | Mutex poison recovery via `into_inner()` and history clearing added to prevent cross-test contamination. |\n\n## Estimated code review effort\n\n🎯 3 (Moderate) | ⏱️ ~22 minutes\n\n## Possibly related PRs\n\n- **#275**: Modifies ScreenIntelligencePanel behavior and test expectations for platform support conditions—directly aligned with the component changes in this PR.\n- **#886**: Alters the `.husky/pre-push` script (pnpm vs yarn transition)—touches the same file modified here for Windows PATH recovery.\n- **#453**: Refactored ScreenIntelligencePanel by extracting child components—relates to the conditional rendering logic introduced here.\n\n## Suggested reviewers\n\n- senamakel\n- graycyrus\n\n## Poem\n\n> 🐰 *A Windows hare hops into the fold,*  \n> *With path separators brave and bold,*  \n> *Tests now sleep on unfriendly ground,*  \n> *While pre-push guards find tools around,*  \n> *Platform-aware, the code now shines,*  \n> *Across all systems, boundaries align.* ✨\n\n</details>\n\n<!-- walkthrough_end -->\n\n<!-- pre_merge_checks_walkthrough_start -->\n\n<details>\n<summary>🚥 Pre-merge checks | ✅ 5</summary>\n\n<details>\n<summary>✅ Passed checks (5 passed)</summary>\n\n|         Check name         | Status   | Explanation                                                                                                            |\n| :------------------------: | :------- | :--------------------------------------------------------------------------------------------------------------------- |\n|      Description Check     | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled.                                                            |\n|         Title check        | ✅ Passed | The title accurately describes the primary changes: adding a Windows dev runner script and updating lockfile versions. |\n|     Docstring Coverage     | ✅ Passed | Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.                                   |\n|     Linked Issues check    | ✅ Passed | Check skipped because no linked issues were found for this pull request.                                               |\n| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request.                                               |\n\n</details>\n\n<sub>✏️ Tip: You can configure your own custom pre-merge checks in the settings.</sub>\n\n</details>\n\n<!-- pre_merge_checks_walkthrough_end -->\n\n<!-- finishing_touch_checkbox_start -->\n\n<details>\n<summary>✨ Finishing Touches</summary>\n\n<details>\n<summary>🧪 Generate unit tests (beta)</summary>\n\n- [ ] <!-- {\"checkboxId\": \"f47ac10b-58cc-4372-a567-0e02b2c3d479\", \"radioGroupId\": \"utg-output-choice-group-unknown_comment_id\"} -->   Create PR with unit tests\n\n</details>\n\n</details>\n\n<!-- finishing_touch_checkbox_end -->\n\n<!-- tips_start -->\n\n---\n\n\n<!-- review_rate_limit_status_start -->\n<sub>Review rate limit: 3/5 reviews remaining, refill in 19 minutes and 28 seconds.</sub>\n<!-- review_rate_limit_status_end -->\n\n<sub>Comment `@coderabbitai help` to get the list of available commands and usage tips.</sub>\n\n<!-- tips_end -->\n\n<!-- internal state start -->\n\n\n<!-- DwQgtGAEAqAWCWBnSTIEMB26CuAXA9mAOYCmGJATmriQCaQDG+Ats2bgFyQAOFk+AIwBWJBrngA3EsgEBPRvlqU0AgfFwA6NPEgQAfACgjoCEYDEZyAAUASpETZWaCrKPR1AGxJcGsfBRIACiUJAEouNFp6AHV4DFp8AHdkEMgKbAxyPkx6NA94IiwPfAYAawAzeC9IKQpEeHwMZEgDADlHAUouAEYABm6AVkhAFAJIWFxcbkQOAHoZonVYbAENJmYZ8QxZJeZMRG0Z/G4yHcwZ7mwPDxm+wcgWgEE8PwouAFkAZiIHsDe4+haAGV8NgKAxvJByiRqDNEv8kogwF40KUwL5RKVIIAkwhgzlInEguzi9wMgNw1Gw034xywowAwgFqHR0ASAEy9VkANjAvQALGBWQBOaC9AAcHA+nI4vN5AC03AhkBcrmkSABHbDSXDoKLINCQWLxBFgcolSnMkIkYrHPiIckUXDYbj2BgUeDcbW4WDUHAEMC0JBMWpKjDcZjoeKQDBxIT6nIKDCVIig6SQOkAUQAYpACPh8hgiJD/DnYCRIJbrWwMNqyBJ4BRGlXNJAAJLavKIfCQJ20JmIGYCRxTSDFMqVarcNBlNCkGqUeqNZAESC9DQDD4aXlFvhMJpIGgYBiyDQwUvWOwCRTyOIMDzYJRLkjMbgeJn0RCicSLyCBQGOXYuAANNYDYCF4zDAcCd5fhgkHLMwSALlgdKlmU+R2sBLbPlOuDATYVpvqEI4kOU7ZKq+4J+B4Sh1JAcJeiC2rjl49BKOSVSICebwkOSvbklwuxKGA9GwGmoKdhQwEYF2AR1iQiTzvwfCvp0HjIOhNC0CedIsAh2psIg+yzggRCwPkpm4MgARQgEh7MsSXpliQAAe+5xIWDhOC4KDxPADBMqqr6aYw3oFqmy68Dx4iUAmSagtQDRYPG+qVM5OZdo5PABGAFyIKJfj4Jiy6dBMMUBEGMWtIoZaNAa8LJJAci+oQAaIBVboFlG1URvQ3ChuGdb6okpYBBoLkkDMR5EJOXo+ZAADi6iQAAQmgeUaEY+jGOAUBkPQ+DlM1xBkMowVrE2XC8PwwifpIqZNUwNEqGomjaLoYCGCYUBwKgqCYEdpBZG+CisOwXBUIk9j/s48gPdVVCqOoWg6Ft22mAYbVuh6/bpBg/okBIwlxBoeUcAYABElMGBYkAPC2x1A8FnkAfIB0hZgpCIAYLbVg2tDYOCepRvJdVGskYD+PA7AWgTVpHE2aQZFkLpY563rah+lnds6dpumIjVraJ40MHgiXAVqKjoaWVkkNw+D1AQ3nJdwzqJP4pTueW9afpL0jAfGu5xQENsat79C1vWjbsDUzjwJbqaBDed4Bp1AAyqcAGpvD1aZZjw1CwIgoQnm2Oq0MgpYeDakIZGIiVLl2o4BQABn1YZjc5JDNznzfRhgsYd13kAkIsMXlA24aZYgsh2k+1gPNAAASSmQhPovzTxjBTmeAblY7UvIBkNGQM3qcAPJ0g8qcPFYVgACILw8zfAeU2j5oW0IUB48giYwyJ8AZIyqZ4CHRHo5PguYPAoEPhgNAEg37xxPHAMsmN3Qax4sgZudI3gPAANLpgAPo4PwQQ2wZ95o2AeG8buy5MrBzzFIegrQYz6mNngeOOddwBhgnkb+OAiBNkwVYBei9u6/0ykFLUXs2r4FqMyDM2ZDQJAarjcQbAvZ738MeVsasyDD07ibGgyBoBoFBPAMAARJG9QCJOKgMF7A0GHINE+bdmDd0CM3ckZiOBkAcAEZ+J8mABA4HaGcXdQjAWKJEFI+ADwSFigUeKMF/aRlCQ6JcZ4TFmPLATewlBajdnqJ1Zu/kKBEAyqYt0OSJBiMWOgewxwGAgL8ifG+VhU6EMBC2earQWytHmgQlsd90ytGgC2aAABNZuG0jDU0sA8DwNA7H13SiWMsShbzOASt+NmLk7YOmZMWC4YEWnsHUAfAwUAHhRGZOQSGbDyRgRQa6NBXBm6oOxjMXG+NCZwgwCTWA7i7mQiqCQYuRhU5xFTL4DmdAuAAGoADsooZg8iMOmO08BdhnW6rJKW9zyimgdO8Og8BHAUyppc9GGgFjiEKP4bwFLyZzNpvTQGp1mTMxhvwQ6MKwpcwAKrcD4qmaZtKCjSX8asiVDK1kn3tPiMARjcAWIyN3GaokfR0NtvbdQWi0j4FiSk+gqVwpnnHGWZce06J1P1Hc/MZY7EjRLP9CiHtOqNBIDM8w8zFmnRWbQneohXzLJ2aA5y+zgpHOWPkBgw9qznOkJcyArQz6tHTBCqFyA+WkFoAi7oqLejosxdi5kj1HUE3xcPQl/gCSpySEyza6M0AuxmIgMEk0WB23INWfsmtNhEH7JOcgakZiAldCQMgPMaBXAKGQcEVhMBWg0JZZyZNKbMppnTBmHL3zQ28mzHNSbm5WEoAhQy9dAS3UaN3VA0kRyNFINkDwiQ0Az3sH4RIGAuBLTvUkVU8RKCHIwHw4aej3nkkdIgAA/BoF81AiXMAIQ4F2ta6C3qXOkL08hAi/31J49IQ8lCv0uNqMDWAIMUkQBh7sgHKjkFoMXWmURzmNF4bIYCmVdgMDPoCCWIH5AZBQ5G5kgCwl0TWqqejzJlyIEaSA+QXofQPlKAQZ0/kPQphzjac9SEEy4AbGpdAARCRTl4/x7+MyWgsoWUs7ZTRVmZQ2SG+zyBdkRrQ/tPgxzY3xvENFLmUAqrkBmZC8g2bQq5vzSisAgxi1qOBuW1UckCVEoJNxAM5KN1NrAEYFt3A20drWN29gfaoruSHUu0dBCCHKsQDVsdE6p3VitOZedJBF0jpXVqFdiA12Nq3Wyk6djOX7tZryyLSb8KwIMnK5VW8PyrOsl4fWwK9mflc9uWjwnPM8CCohzip5LVSOkpDV96hkBEvqXJ0QzS43cfM40PhQmnQidoDlfb/hwxidnMufLn9jXoEMpQLWKgPx2R5XKs5JmdOIRWR+OutUmqIHor4T248WAn3o7QZashoAuVwPMHiuP8fOUJ/iXHNg8xD0CJFcH4IQoYiLqs5uGpKCyBJwTmYbOXCU+p0CxoOVg7tcZ2UIuwEk73k9gEZgsi8iQ+1Wk7UyZ4C9ghzQMnOdMqABwCfCSvIAADJID4XHtIUSp6KC6froAXAJGp4AIFgDHk8zx3zPm8b1Nm/Whoc4G9ZwatkwTc+Gt7K8fOnITQF5NwWyyBGVRZ+QR6i6ZvC+zMKebIDwu6KyVFop4tYsS7iytIsSJpZJZl5gjaqW5Yxh2o4JxHBnDCdWGY3oKDhbKx4coBDSx5HchoOo66qaDZ3SNvdXlxup85gYZB8rDUeAIbsUolACG8C7bgAhkuHwEPOjkGhJ3/2St2PkAAXmass7LR9ZTX4ATAIlQFzybY6g/gZDyBsRRT2AgpylEQK+PKqZf5EqvoUDvi/7WyNQkSyq1AKaexKa6I8B5iyDMT5yzS7jsQObNywjuxyZTgTR2zfzMT9hCBqjTJHZZSVogh6jA4OiJSi5L70CZQZCy4BiVDMir7PgawGaey7yfjfzASL4wFnjzbg4Ox3SrLwa4CIZgA3ZNKVBxoarbjYoDoe5bpe6ba+45KbLe5B76Ih7RonJxpnKR5BaeozI0xvCYAgJSKZigq0ywLfyn4UDJ7QqTbp7wrZ6xZ56lr0BJZ4rF41rEqQAZZkoV7ZZV5GDtoMCHA0inAYCdrPi6r4DnCUANABgMD97TADbzJDaMyjbj6Q6J4KjHZ2gny4wEKeq1Z+SlAEIBCOht71aFQELkYELSTb75DsDdynb6KiB4CiqlHlHiBlCBChDdxHwxT6iDBSGiCND0DQCFQNA5hYokCMQ5xrQfjpIoDajFZeBGJpAFwxRKZYB2wHjiDsZjAcyez/AkRxDqBWjaJWB4oUG8EbHoDnbBTNx9HkAVGDHDEaI8HyDxgyoBD0FnjBykYADccqzc6YFAFA3c/ki2r6yA0kZEaxmkJ4VUV+/MiORQ+ACwchBcyACkJmR6WksynudmgejmQamhm27muh3mMa4e/mFyUenqP4T2/xOYUil4tACeLhEJ96YekReyu29QhQFIwchIigzSdA4KLK5h0YUIxR1h1QDwdhsgDhThEWsKrhAwqKvInhBeSgyWVaJetaZewRleEA1eERUR9euwsRxWCRGwboRAT63e+4Wi6Rg+m6WRI+wMXKB6E2sKXM7xUxwBtWrpT6HR/6yYzg5cIKXg08s84Yo4pQMwGQaZ1I/q34v8zcZgAA2gwOUEQIEMibhvVEXKEAALoBKdgPplDoAMChwiGNDASZklCYhTgtmsZYCUANh8ChS0DvyA4dmNmgmLJX6ThED2bGYoIewuzMjQ63H8BYBKIIhIJniJBug0AzDlB3h5TIGaqRiZQGZTjuQzDsDeQTqznFB4mqhEhYC1wuHKG+oUkBoZTUkuaUl0m7Z6G+aGEXLGEhZamT5woZ4fCFqGk4rGm+GpbmmBGkpZaUrWnhG17REN6OkNixFtSlj8xeAUC1ZaicQD6ZGsr+lMxjb5GTZczT6CFSIOA3FdSQxcK9mnHFagqXaiGzEeyECf4fjvjoj4UxRCCCBdGGI0F1arl8K1TSR4zrkNTiEHaNT/EsbFKFnFmlnll/LKJVm1ksicGDg7EqCyKWqlj1g1yHj2LEbXGB7oldixLOrzZ3ktLFirEg4rJEllgkmvm0yqGUnqHOYB4rK/kHJeY8CMkGER4snAUx4ckJjcKJTy4cVBQ0HUCGU9GElAZlwyZdi8CSABTzblC1yB5ylhbOE6kIp6looGAYoJYwUVopbVql6QD1qJBWnoy2l14YAxEzBNzz4HAaokUZHZbD4X4BlUWHo0V0VlieJ4g8Qr4Ekr7C4UBSAEJg55g9FlG1BugPh77FGdHmjkQIZfZgD5YNj5UBQbXQRljyG6wVYqV5IrYDqiy6VSG4CyATgEl0SlgUaaUgAVlixFw0YGaag5wCrRjOTvWfW3XfWOWUBwgfjaT+DBzdopyFjuXUG5nZU9jAzLjFbOBmXeVdo9H0HzXahDXoAzlxDFEnoFzLTYDlAcAcBO6BB2i4A/Hxh01egM1M0s0Tyx6WQ/E000CRCQ6mr0AYG4DPgzAaBy3dz5B2ZqS+W2Y5k+6fl+40k/nB5/kMn6F+aJqBYppsmBDxXOW4ktI02q6zUaUlmx5ag1lSn0HEViKSZMEymMahZZpgWuGDCordDdDQVlqF5NVmkBFBHIXMphE16RHdW9UI5mIfXnB5h+SyBEV2jDU+k2bZG7pQx5FTUhkGBCoireGoSlAaRHlgBgSdkwHEWrKdB7YnWW5nVAFeqkHzbwmphXbV1lDMgNixI5w910HoACCdg3VHlIn/oCBUCHiiS1QlIlkA06UIjDH+xUEvUKWIh2gw0j1j09ET0/hYIcAAA6ctGgzcREhNOS0YwGr1CIP1NhqBcQ2AMBXYmN2oEN8AUN291Q11+9lNHist8tl9JmSgN9+0WAclYAm9jdEhX2h2GJ8NEC3JJA3odYxYxYLlcaiJPtKt/lH5cqQVWhkOIpYVoekVBtRhxtIF8pFhSp2oKpZYapeQGplAoFJJ+a7hHwQd3hIdpp/h6WSFIRKFnV6F9pZwiAHsVw/YRw9WdW3pZF2641lF+dwZ/KyaxdfYkJjItAyGUj8+9CoI4I1RJAIgYg9Wf9NAi1Xo1GXJxRy45oEYkA5m51RwboV1o9m1NAR54MtsFEqYKUX9zIn9aU8hgDPEkRk4hkiQtAF9Nqs0+G/1gNulPxrFPCUCH4z1ns+oMDljsNs05GismQnstUm9G0UAuCk6w4U8aA6i79NBnQaDDQfAyO4gKo62dcnU/0/ZxYqB2gfchY2ueTtuTuJR0IujkjVQBj0gIIYIQ8hTq+dYSguQSUnj49GqeD7534gV/uxDoVUaetAF0VSaQWaaGaBg5V2paeCKiKqKrIPDCgsFRe8FARbVHVNpYjPVmFGwc+/YWKL4A4DYyQlAbaTWTQfgmgpFo1fpyjuRLM1Fhd0AbstGS0UlXlTtHtqyn838FiPEoIWAslguzc3G9sGBUDzc+Y2Azk6qn2luiAwEkUUgCanUjk4YozDyJTT5TQr2u2vG0gm5RRlk1+CgJm9T348YWDD5/TtGJJP1eiuMq5pmPGgIK8YWVLmzatDchDuztJOtZD/5TJhtrJNDlzPtCKnIUFtVJaRpjV/DLVEdwjUdqFMddpXzDpPzeYfzz41wrojQXyGQCj0L5FsLY+8LBd6jUAmjOxmUvrWACrkCdj2ozcRKxjuM9WolAg618QJjQS5cHpdoWiNCXYQ51QMDjTcCzTXshKQG1Y38XApTlZduVQ5cEu2onRHTWsGb4lpstUy4r8VQwEUBLBGSZY/bd4JmjEFw2om+oq5M2BX6w8MJ/g5MAS8YLkF2c2Ui2L14TQot+0h09TnUDgDAgsiAxVUCHLtU8Y5U/gyzYwnpLgGrxDOzWtIVerBzEV+tgFJz1DXq7DLhCKgolrdV+eDVJpfh9rQj7zaFsdGF7rkCXrAL5AuAbsFA6ZJsX8gbQ+MLw2E1qjYFXM+OGdzFXUluvep+R5D+Aez+j1u4tQL1mA8gzcx9t6FG+w0Y4gp+BCD46+DgAgu8gQZ9Pxk7eALOMw3cnQRKZYh7H8Ean4nKnBnU8Cd4/Lp4f01b5jcqYrDmV2eTOU9+91Ls7kDLhN1YkxAi0c44SyxnOcS+tsnsh+8uliCUUgE9fBKIAhc8oElInoeVtLzAUh8mshVHdixYzBNk7WnEZJKhWz6t2rr7YaOhutn7RzzJP70eP4iec5CSDGjmUilc1c8Y822nhJiwxIxLigib1GcpZhdDVhNhzD9hbDFz3tHDGeWeqK3DVr9VwdTzodAjFpkdOWMHrrvVCHMw/z1wyZNA6w0kSgBCxsWHvpwbuHKjYbajU+RdwqWjmUYZY9a1Hy6+GqJjZjlk61azPRIxHHibqyjj+oSlp1aArdu9Xj+TokyjxYHieTyGtTL44SPks8YtbMgTnc9AVgvGLYAAGkeZuagL4imCO5ANrgEKdy9+PYd9Y7ALbtp9Al0WCEgMyHhqs3vTQPp7NPdV0y7Bde494+oRh7ZNqLxj+DAxIJPfJZWXKT6n5bF1q05jq9rUl/q4c4a5HsazHll1fYHFCvQRlFqPHmMFaDaKV16OV7baWcqjWd3EwZcGCqYZYAqZYcqQ1+qZqS1ynm1/CjFoHd16B717axBwhW86Ec611XB2cON5N22jPDNzMP1AtwYkt9nRRXC9yuG1Ptcgmfd/5/jDSGA+2Bd6T3db99UFdlJU1HEAZooALJ7M3N9/sN60ED8XGcAcyE1AvUQEvRzyQTPntwwiQNvjEyd5+BYwn/XzkItQz1d6i1IqgO7SwTL4UmZagAV2VHsRAqFPUq3u9uWpLeTJebgAwCuyODcVQB4ObNyx1IWDrij5+Gj/vQwDE9j+vQ06g3AtCouPuOwHwlOA2IZHfQ1PGFAzAxHNhYIieNZuSZq1SZrd+W+4Lx+0KUoZAVf2mXGitlyl65d1C+ZIsnbQ176VteXgMqq1wA4Z4reDzHws82aoIUHW0HF1nHW+Ye9vWXvFMm2krgeBA+Y1VbiHyDIEdp8ddcrn/iuDpFu4V9XGsFGKhOQI0saTwPICf6NsPEavMsrEmSYr1NeREK7B+FqDy5GBHgKuqf3QY7g1ofsFYsf3FYmZdYZAIgI5EgFdgZ2W8TTECQ/StZ+AeAKdofXJgABvAAL7kxgIE5TQCJ1wBDE4mhTTKD0woDvUAo6IMXJCDfgCsbuCdHcvIDwJ+QD42XPvlLAH7T18AkQcDLwiSB0Bt8ukHILYyaiRB0aJ8TWCwMjDEtSgu8avmeBvgthIAS+eQMiFKBhJgIi8N3OmBmDCIl4ZBNYvAmSQ5w2A5eCxEgC7JU85cUCKSp0Uya78p4ZAkGA6QH4KtnED3ZukoFj7Rx4E7jJ5MgA8S1hu4rPTITxACTNwAAJNULeDpgMCWw+oaIhqCYJNhtYDgAKkBDpgbAZCTMC2A6T7DThGACQBwEOGrschBAAWICmOEnxmAeQ+sHE2Si0AhAPnP+NCAwBOgH095KcLm3ch8JAgFqB8j0O+HcExA+qGXD0LlIkhP+z7DWhoV/6JdSGAAiht+yNoZdAgEvYko0GkwD8O63UESOVyYJVdWOmQz5mN1+YTciB03J8KQNazMCau+vOrkb1VIm9muprC3h8BiysguuIHLwo83t4vNBG5eXAXVhmCmICAWxHiPXzYCy4XAC3VkF6ihbYcVuORUNqHw25JoZqVXEjjewKSjN7u+AJAJ6klojIM4BCc+HSFwRa8eiaUJqATioCdMt+Z4OIMrGL70ARa4zcWm/E9h5kO2JBCPmxSuAcZh428SAOmFZDpgbunQBYA5jSGlR2Cr9UEc4AcQMpcgeAEoF2m2JlhFQjsHDM3GrFaJmat4T+PmxrFDEtArxAEaRBiiOjUqLLM8M3B2F7D40dYV/uwAZayQzknUX1oZCVRSI+mCEWBDBBPDQkByl2NeCUn/gtjC22XQEsyA8TbFIABCSAAAF5IAZ9FwQgGqCXssAT9cEfy2i5vkv+L7fEQ5n2aHJheUVNLrRVio/h70hIugOcAoZENNs6LEkvyMCKCiGGxvFhqbzFEoD4U1Va3jKJtbgcFRg3R1sNwMAaAlgkjWQOcGyi5RYAWdSgSaLzrrdaBM+SKDlEpD5RDUmITojaGUqb1Aut2YLocNVDtRYYEBEzLjAGYRg+STOR6k3Beq9xqog8bIZLVcTiSagccE+GBlGjjQJJZBWPp52kTtQLQ3sVEW6HChdgT0IiOMV2OyB5iPQ0uKYrUFZhrlKyrbMuHqD7JcDQhNYZwHwjHZaZf45oDwa/CaSdQfsATHIeu21DdAxEv1E+HNyHioBMUKoDIHAgQRPJxxssZlh5GWAfg2c1YFxP1GbhV0lBJqL7DCHzCE4UqE0GwCCO8Hf5V4mOXidGMWDLEqoSgJ9moVxHASBe/48KoAJJFi8fwMbMAVfXyB6RcqKsNBGAAz6UA7CRglUMVSsorIA4jQTPlAn3L/pyu2EykKUDwlUTCJF9L2ub3gm8gC0NVZCWBzgpYDw6UHZ3s2lbRwYai0UV0pKkZRBslGVA00TQIKJQB0wZOPaJgnyyy1IoEwKWFdIZQSdVKGQ3bgqh4izi7QqqDAN3CvJJj/IlIT2PcXKz7EuwkzZ0IhnSoXlV8p7AQspk0k1jzgBcOqQFQan88/+zU8hl+2OakjPU/7Squ12A7Wt9pmAsOnWgbQnTDABgL6PGn3YAx7pIwsGLsUhiBlYYCeeGM9CRhvQtoHM86OoA3x5s4KiQ/XKjA5mshEUtAAYLQE5AMBBQJAQUP5FFB0AGAoobaTgQ5CCgPgtAE9r0AGBazeQaskgGrMRTvRPoO0SAHrNEC9BOQooAQFKORQDByg6s3kLQAECCgBAAwTkKyF5ACAJRJAXkKyAYAhzewnIWgIigECOz2ZzstAJyAEDlBBQ3QWgLyGhEqzRQooTkCQElCcgPgHwYsgIF5DQgPgYoT2SQFFAJySArIVOUrKDm+zOQYcgYKyG6CdBEUAwXkHHKTlBzugooA2V3I1m0BWQrIAYNrIYC9BaAvQVOWnIgAjDpZquerHLN0bWoJZzsyKAvkoCkBt8ZderArI+hGALBBge4OTCQC2BloaZOgDpFBjVgrA9sTSOTC4Cvw1IJAQCNfMgC3zEAZ8HaqrjAafzfBP8v+TfISAMAKeRAHSFINIDTphpeQMkEyHAVXz7gN8w7jjAyA/IiY/yPKBgv/lYKAFBAckB4EzAlV644Cj4FAtIUALxp2JRALEC9B3wSgcCxAOAqLSkKrB9C7BayO+ZN5CcredvG2itBd4e878dIsQoYVkLYkeQKhRNMXDgLug/CrBeTCYWB5WFsAdhbAsU6DpVFJCyAHwuMVztBF7rJ0guBdIFB3S9Yx9nUFkUMLyY5CxRdQpUU9B1FN8rRfXB0V6LOF3C4xaYtIXmLYO4jR0l2mdI2gUifkGRVwEwXOLXFlC9xU0FUVeLGFKSlhYsH8UGKuFPQIJeotCWjdvm/VdavADxk2M4lkABJSEqSVKLmFaS4xd4syV+KOFuSoxbwsKWu9wlk0bCqCzwo69CK8jRxfEqaXyKKF9SwPOAsRTpLNFLS7JW0s355LIAAdApWYu6Vut3e7Iz3tEOBYUBQWAQXxBCyqU1KNFdSzJeAtZCzKfFi4VpfoqWWXK1lISjZWyM9YciAWsbf1v8hGXVKxlLihRckuUWpLPFfym5U0DuUBL8lnS9ZRYokbdEghydWNGnWGXLLTlN885UCuWVXLQV8ythYsoqyPLoVzy2FThX0YyMpg6dSyCcr+UYqGlIKuRXMsxUQr2lUKrBcEo0UvKCB2yzkd725GhT/eogalQytpVTKuAdCnFUyoWX3KCVXAe5kSo5UkqPWo6T3lyNm5hgBVaRH5WivGVuLMVtC65bit0X4qCwWKp5QqrCWbLYihApDjxFQ7odQQ5ArVTSoBWTKaFXAAYAaslV4rpVJq2hWavRXEVVRZYjUVY21FaI9RBo1Fc6omUXL3Vnq5hcyoeVcBc88qgRRateXKqeVJAmQUKsSUurY1kAGZRKoTVSrIVkAQUEEv/nsryY5aGwKLNwDRAGwNAe4u4FwBeBwF38j8PQrnZ+BLgOONMrYA7Udhf5/88mAGFoBFSMAeiskEspQgYhwFoNEddAtVyTrW1XgOdWUAXWEZu146ydXfGkAvITJjQDdaUCHWQLR1+YOgi2EMiahEAM68BZTG7W/5cAJ6vXKRmWUFljF2qmtWXVaC1MSAD6/dYdxoInrbBfy0JFBi3WahZleyV8IuMSgPqT19gBcscHoBQAdISgOtYjFwC3572pkJELLAyZUU4esCJ5FpDA0MqmCAGrgOTCAIDMKNziyWNmLyAnq/1bAB9Q+EPUwRmUqa35QypKlsbqNACtdd5TLoMbnlkGSkFBqXXOLYNmAezA+pnz+Zf6J7JJCuU41uhOgiPfKvC0TwRA1KGNe/ikFyS8SYoh3HOGwM9hpkERwYeuBoHE0aKqND6uje5Ac03yb2iYRJAEDPVdq/lTGuICxt/X/qH1ymgDf6r43OKBNwWmjTks35phTKVAUgG5oAUQapNXARdTBq4HybuNMW41YWHajiZUAfQVcL0F6AABSB+r4Fx4OBCUoQ9gP4ICChxDBXoYOFRDDHIBRQJW8rfZtmVOaaNLmgsMlvJj+a7CrG6LQApgUBLwt36qLexpo1hZh616hwM4XnWzLUtyyjLX8rk3wbGgiGsushvdCobwCMMxbPekvX2Qb1/+bKqaCPhbYVeSoS4FAka23rNAQ2vrQAoG1EAhtI2wLRiEE0PrzttAJbbeoeDA5DITYHjWyvUUzagtc2gBWfFE5sxx0deNMGAJKlDb1t0mzLfBh20YA9tGIA7YuXoCdATtZYM7XEGHqIRb1dEa7SCEjCp9FQEVFUM9p6xvbqozm5wPRtmU/aPAY2uHcNrwBnxygyO44ChBDKg6Pw4O9gJDvuBWD/51ZJ9WtFwC2AgNXGhDTRq1kfAI5ooRFNHL7lRAtZiKGOeUAYAfA0A3QGuWgFFBoBRArIc3YSjQC9ASAQczWdnPKACBrdQoAQN0AYBjyBgAwQ2R8E/wVyGN5MZ9bYBE0PrOQhKS2S3PKDuyzdooPoHQERQGypRaAE3WgF120BRQgoROTrrtkB7egz0bWbHM62667ZQoc3crIGAGyPg3QZlHLtXlQAD5bAMpPXxKn1Zd5F8jmWqPwCLVzQyGSDPX3PlsyLBYepXYunNC0AHguAfCClifm6R1AOkDILgE/m9Am9vessQPo/BD6mQC3SMPoCAA -->\n\n<!-- internal state end -->"},"request":{"retryCount":1,"signal":{}}},"response":{"url":"https://api.github.com/repos/tinyhumansai/openhuman/issues/comments/4342093632","status":401,"headers":{"access-control-allow-origin":"*","access-control-expose-headers":"ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Resource, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, X-GitHub-SSO, X-GitHub-Request-Id, Deprecation, Sunset","connection":"close","content-security-policy":"default-src 'none'","content-type":"application/json; charset=utf-8","date":"Wed, 29 Apr 2026 15:01:16 GMT","referrer-policy":"origin-when-cross-origin, strict-origin-when-cross-origin","server":"github.com","strict-transport-security":"max-age=31536000; includeSubdomains; preload","vary":"Accept-Encoding, Accept, X-Requested-With","x-content-type-options":"nosniff","x-frame-options":"deny","x-github-media-type":"github.v3; format=json","x-github-request-id":"9E19:3EA327:299AA5D:A6C977A:69F21D3C","x-xss-protection":"0"},"data":{"message":"Requires authentication","documentation_url":"https://docs.github.com/rest","status":"401"}}}

Copy link
Copy Markdown
Contributor

@graycyrus graycyrus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR adds genuine value — Windows dev support and cross-platform test fixes are both worth having. A few issues need attention before merge.

PR description is boilerplate. The Summary / Problem / Solution / Impact / Checklist sections were left as template placeholders. Please fill them in — reviewers (and future git log readers) need to know why these tests were changed, what broke on Windows, and whether the locking removal in trigger_history.rs is safe.

Three substantive issues flagged below, plus a few minor items. The biggest one is the scripts/run-dev-win.sh path hardcoding macOS cache directories while targeting Windows — that's a copy-paste bug that will confuse the next person who tries to use the script.

Comment thread scripts/run-dev-win.sh
Comment thread scripts/run-dev-win.sh
Comment thread src/openhuman/tools/impl/system/shell.rs Outdated
Comment thread src/openhuman/composio/periodic.rs Outdated
Comment thread src/openhuman/agent/harness/self_healing.rs Outdated
scripts/run-dev-win.sh:
- Add cygpath availability guard before to_unix_path; fail fast with clear error
- Extract shared find_winget_exe helper to eliminate duplicated WinGet lookup flow
- Pick newest WinGet package dir via sort -V | tail -n1 instead of head -n1
- Fix CEF_PATH to use Windows LOCALAPPDATA instead of macOS Library/Caches
- Replace bare cargo tauri dev with $PNPM_EXE tauri dev (uses vendored CEF CLI)
- Remove macOS-only APPLE_SIGNING_IDENTITY from Windows script

.husky/pre-push:
- Add has_pnpm() guard after Windows recovery; exit 1 with actionable message if
  pnpm is still unavailable so callers don't see cascading noisy failures

src/openhuman/composio/trigger_history.rs:
- Add WINDOWS_TRIGGER_WRITE_LOCK (process-local Mutex) to restore write
  serialization on Windows where fs2 file locking is unavailable

src/openhuman/tools/impl/cron/run.rs:
- Remove early return in force_runs_job_and_records_history; verify history
  persistence on all platforms (0 records on Windows spawn failure, 1 on Unix)

src/openhuman/tools/impl/browser/screenshot.rs:
- Remove OS early-exit from screenshot_rejects_all_unsafe_chars; keep test active
  on Windows, only gate the backslash char on macOS/Linux

src/openhuman/tools/impl/system/shell.rs:
- Remove dead cfg!(windows) branches from #[cfg(not(windows))]-gated
  shell_preserves_path_and_home test

src/openhuman/agent/harness/self_healing.rs:
- Normalize prompt once at top of tool_maker_prompt_includes_command test;
  use normalized for all three assertions for consistency

src/openhuman/composio/periodic.rs:
- Reduce run_one_tick_returns_ok test timeout from 15s to 5s for faster CI

tests/autocomplete_memory_e2e.rs:
- Replace let _ = history::clear_history().await at setup sites with explicit
  if let Err(e) handling to surface setup regressions without failing the test

Made-with: Cursor
Copy link
Copy Markdown
Contributor Author

@M3gA-Mind M3gA-Mind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disagree (CodeRabbit shell.rs naming): @coderabbitai suggested renaming home_cmd/path_cmd to camelCase. Rust naming conventions mandate snake_case for local variables — camelCase would be a compiler warning. The dead cfg!(windows) branches inside the #[cfg(not(windows))]-gated test have been removed in this PR commit instead, which was the real underlying issue.

Already addressed (graycyrus shell.rs fixture): The comment noted test_security_with_env_cmd() was missing "set" and "mkdir" — the current code at line 298 already has allowed_commands: vec!["env".into(), "echo".into(), "set".into(), "mkdir".into()]. No change needed.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@scripts/run-dev-win.sh`:
- Around line 16-19: The fallback to APPDATA is incorrect—ensure the CEF cache
is always rooted under LOCALAPPDATA and fail fast if LOCALAPPDATA is not set:
update the code that sets CEF_PATH (the CEF_PATH assignment and export) to
compute the path only from LOCALAPPDATA using cygpath and, if LOCALAPPDATA is
empty/undefined, print an error and exit non‑zero instead of falling back to
APPDATA so the script does not use Roaming storage.
- Around line 87-93: The script currently sources ../scripts/load-dotenv.sh
after Windows-specific path adjustments, which lets .env overwrite computed
values like CEF_PATH, CEF_RUNTIME_PATH, LIBCLANG_PATH, or PATH and break pnpm
tauri dev; move the source ../scripts/load-dotenv.sh invocation so it runs
before any Windows-specific path computations (i.e., before the code that sets
or modifies CEF_PATH/CEF_RUNTIME_PATH/LIBCLANG_PATH/PATH) so those tailored
values are not clobbered, leaving the PNPM_EXE tauri:ensure/core:stage and
PNPM_EXE tauri dev invocations unchanged.

In `@src/openhuman/composio/periodic.rs`:
- Around line 283-287: The test currently only asserts that tokio::time::timeout
returned Ok (i.e., the future completed within 5s) but doesn't assert the inner
run_one_tick() outcome; change the assertion to inspect the timeout's Ok value
and assert that the inner result from run_one_tick() is also Ok (for example by
matching or using result.expect("...").expect("run_one_tick returned error") or
equivalent) so the test fails when run_one_tick() returns Err; reference the
variables and functions run_one_tick(), result, and
tokio::time::timeout(Duration::from_secs(5), run_one_tick()) to locate where to
add the inner-result check and a clear failure message.
🪄 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: a0fc7615-479b-4f11-bb38-4afdefaaefa7

📥 Commits

Reviewing files that changed from the base of the PR and between a6bf91d and d9e28a2.

📒 Files selected for processing (11)
  • .gitignore
  • .husky/pre-push
  • scripts/run-dev-win.sh
  • src/openhuman/agent/harness/self_healing.rs
  • src/openhuman/composio/periodic.rs
  • src/openhuman/composio/trigger_history.rs
  • src/openhuman/cron/scheduler_tests.rs
  • src/openhuman/tools/impl/browser/screenshot.rs
  • src/openhuman/tools/impl/cron/run.rs
  • src/openhuman/tools/impl/system/shell.rs
  • tests/autocomplete_memory_e2e.rs
✅ Files skipped from review due to trivial changes (3)
  • .gitignore
  • src/openhuman/agent/harness/self_healing.rs
  • src/openhuman/cron/scheduler_tests.rs
🚧 Files skipped from review as they are similar to previous changes (6)
  • .husky/pre-push
  • src/openhuman/tools/impl/cron/run.rs
  • src/openhuman/composio/trigger_history.rs
  • src/openhuman/tools/impl/browser/screenshot.rs
  • tests/autocomplete_memory_e2e.rs
  • src/openhuman/tools/impl/system/shell.rs

Comment thread scripts/run-dev-win.sh Outdated
Comment thread scripts/run-dev-win.sh
Comment thread src/openhuman/composio/periodic.rs Outdated
scripts/run-dev-win.sh:
- Source load-dotenv.sh before computing Windows-specific paths so .env cannot
  overwrite CEF_PATH/PATH/LIBCLANG_PATH after they are tailored for Git Bash
- Fail fast with a clear error when LOCALAPPDATA is unset (instead of silently
  falling back to APPDATA/Roaming storage for CEF cache)
- Remove now-redundant source at end of script

src/openhuman/composio/periodic.rs:
- Assert inner run_one_tick() result in addition to timeout guard so the test
  actually fails when the function returns an error quickly

Made-with: Cursor
@senamakel senamakel merged commit 40fddf5 into tinyhumansai:main Apr 29, 2026
9 checks passed
AusAgentSmith pushed a commit to AusAgentSmith/openhuman that referenced this pull request May 23, 2026
…umansai#1015)

Co-authored-by: oxoxDev <164490987+oxoxDev@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix Windows installer permission/locked-file failure for openhuman-core.exe

4 participants