Improve reset local data guidance for locked Windows files#1811
Merged
senamakel merged 3 commits intoMay 16, 2026
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRust now detects Windows file-lock errors and returns formatted messages during reset; frontend catch blocks display the caught error message (or a fallback when absent); unit and UI tests added/updated to assert the specific and fallback messages. ChangesData Clearing Error Messages
Sequence DiagramsequenceDiagram
participant WelcomePage
participant SettingsHome
participant reset_local_data_for_paths as ops.rs::reset_local_data_for_paths
participant Filesystem
WelcomePage->>reset_local_data_for_paths: request clearAllAppData / reset
SettingsHome->>reset_local_data_for_paths: request logout & clear data
reset_local_data_for_paths->>Filesystem: remove active workspace marker / delete local directories
Filesystem-->>reset_local_data_for_paths: Err (locked file on Windows)
reset_local_data_for_paths-->>SettingsHome: Err(formatted locked-file message)
reset_local_data_for_paths-->>WelcomePage: Err(formatted locked-file message)
SettingsHome-->>User: display error message
WelcomePage-->>User: display error message
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
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. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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]](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 (1)
src/openhuman/config/ops_tests.rs (1)
118-122: 💤 Low valueConsider testing both Windows error codes.
The test verifies error code 32 (sharing violation) but not error code 33 (lock violation), even though
is_windows_file_lock_errorchecks for both codes. Consider adding a test case for error code 33 to ensure complete coverage of the Windows file-lock detection logic.🧪 Suggested additional test case
#[cfg(windows)] #[test] fn reset_local_data_remove_error_explains_windows_lock_violation() { let err = std::io::Error::from_raw_os_error(33); let msg = reset_local_data_remove_error( std::path::Path::new("C:\\Users\\me\\.openhuman"), &err, ); assert!(msg.contains("locked by another OpenHuman window or process")); assert!(msg.contains("Close all OpenHuman windows and try again")); }🤖 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/ops_tests.rs` around lines 118 - 122, Add a second Windows-only test that mirrors the existing sharing-violation case but uses std::io::Error::from_raw_os_error(33) to exercise the lock-violation branch; call reset_local_data_remove_error with the same Path (e.g. Path::new("C:\\Users\\me\\.openhuman")) and assert the returned message contains the same guidance strings ("locked by another OpenHuman window or process" and "Close all OpenHuman windows and try again") so is_windows_file_lock_error and reset_local_data_remove_error are fully covered for both error codes.
🤖 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 `@src/openhuman/config/ops_tests.rs`:
- Around line 115-126: The test function
reset_local_data_remove_error_explains_windows_file_locks (and surrounding code
in src/openhuman/config/ops_tests.rs) fails rustfmt; run rustfmt (cargo fmt
--all) or apply rustfmt style changes to this file so the function and
attributes match project formatting conventions, then re-run cargo fmt --check;
ensure the #[cfg(windows)] and #[test] attributes and the function body for
reset_local_data_remove_error_explains_windows_file_locks are formatted
consistently with other tests and the reset_local_data_remove_error usage
remains unchanged.
---
Nitpick comments:
In `@src/openhuman/config/ops_tests.rs`:
- Around line 118-122: Add a second Windows-only test that mirrors the existing
sharing-violation case but uses std::io::Error::from_raw_os_error(33) to
exercise the lock-violation branch; call reset_local_data_remove_error with the
same Path (e.g. Path::new("C:\\Users\\me\\.openhuman")) and assert the returned
message contains the same guidance strings ("locked by another OpenHuman window
or process" and "Close all OpenHuman windows and try again") so
is_windows_file_lock_error and reset_local_data_remove_error are fully covered
for both error codes.
🪄 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: 7b77b938-77d6-4961-9a49-2548a3af2fcc
📒 Files selected for processing (5)
app/src/components/settings/SettingsHome.tsxapp/src/pages/Welcome.tsxapp/src/pages/__tests__/Welcome.test.tsxsrc/openhuman/config/ops.rssrc/openhuman/config/ops_tests.rs
Yong-yuan-X
force-pushed
the
fix/windows-reset-local-data-guidance
branch
from
70b24b5 to
719d9bd
Compare
# Conflicts: # app/src/components/settings/SettingsHome.tsx
…earAllAppData Adds tests so the new error-surfacing branches in SettingsHome and Welcome clear the 80% diff-coverage gate: - SettingsHome: rejects with Windows file-lock message + empty Error fallback. - Welcome: empty Error falls back to the generic guidance string.
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
2 participants
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
.openhumanoractive_workspace.tomlcannot be deleted because another process is using it.Problem
On Windows,
config_reset_local_datacan fail when the.openhumandirectory is locked by another OpenHuman window or process.The current error path can result in generic UI messages, making it unclear what the user should do next. This PR addresses the user-guidance part of #1615 by giving users a clearer recovery action.
Solution
The Rust core now detects Windows file-lock errors using OS error codes:
32: sharing violation33: lock violationWhen those errors occur while removing local data files/directories, the core returns a clearer message asking the user to close all OpenHuman windows and try again.
The Settings and Welcome recovery flows were updated to display the reset error returned by the core instead of replacing it with a generic fallback message.
Submission Checklist
cargo check+ Vitest onSettingsHome.test.tsx/Welcome.test.tsxcover all changed lines (PR is +63/-10).## Related— N/A: no coverage-matrix feature IDs were changed.docs/RELEASE-MANUAL-SMOKE.md) — N/A: this is a targeted error-message path change and does not add a new release smoke surface.Closes #NNNin the## Relatedsection — N/A: this PR addresses only the user-guidance part of Windows: 'Reset local data' fails when .openhuman directory is locked by another process #1615, not the full issue.Impact
This affects Windows desktop local-data reset behavior.
There is no lifecycle, database shutdown, migration, network, or dependency change. Non-file-lock removal failures continue to use the existing generic removal error path.
Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/windows-reset-local-data-guidance70b24b53Validation Run
pnpm --filter openhuman-app format:check— passed on the resolved files during maintainer merge.pnpm typecheck— passed (tsc --noEmit).SettingsHome.test.tsx(26/26) andWelcome.test.tsxpassed.cargo fmt --checkandcargo check --libclean.app/src-taurichanges in this PR.Validation Blocked
command:git push -u origin fix/windows-reset-local-data-guidanceerror:local pre-push checks ran full formatting and modified unrelated files;rust:checkalso failed locally due to missing/invalid local Rust/Tauri build dependencies, includinglibclang/CEF build issues.impact:validation could not be completed locally, but the pushed commit remains focused and does not include unrelated formatting changes.Behavior Changes
Parity Contract
Duplicate / Superseded PR Handling
Summary by CodeRabbit
Bug Fixes
Tests