-
Notifications
You must be signed in to change notification settings - Fork 6
[feature] Inventory inaccessible handling #236
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
…Accessible) with rules/UI safety
…r blocks, and UI icon/tooltip
… repo null; remove flaky target-access test
… (Windows ACL + POSIX chmod), with cleanup
test: add tests
test: add tests
test: add tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements comprehensive handling for inaccessible files and directories during inventory scanning. When the application encounters permission issues (UnauthorizedAccessException, DirectoryNotFoundException, IOException), it now continues scanning while marking affected items as inaccessible rather than stopping the entire inventory process.
Key changes:
- Added
IsAccessibleproperty toFileSystemDescriptionmodel to track accessibility status - Implemented robust error handling in
InventoryBuilderwith try/catch blocks around file system operations - Created condition matcher abstraction to support synchronization rules
- Added UI indicators (warning icons and tooltips) for inaccessible items
- Implemented validators to block synchronization actions on inaccessible files
- Added comprehensive unit and integration tests with OS-specific permission handling
Reviewed Changes
Copilot reviewed 51 out of 52 changed files in this pull request and generated 19 comments.
Show a summary per file
| File | Description |
|---|---|
| FileSystemDescription.cs | Added IsAccessible boolean property with default value of true |
| InventoryBuilder.cs | Refactored to use EnumerateFiles/Directories, added try/catch blocks for access exceptions, mark inaccessible items |
| FileSystemInspector.cs | New abstraction for file system checks (hidden, system, reparse point, etc.) |
| ContentIdentity.cs | Added access issue tracking with AccessIssueInventoryParts collection and HasAccessIssue computed property |
| AtomicActionConsistencyChecker.cs | Added validation to block sync actions when source/target is inaccessible |
| ContentIdentityViewModel.cs | Added HasAccessIssue property and UI logic to show warning icons for inaccessible items |
| ComparisonItemViewModel.cs | Changed collections from HashSet to List for sorting, added sorting by inventory part codes |
| InventoryComparer.cs | Added PropagateAccessIssuesFromAncestors to mark items under inaccessible directories |
| SynchronizationRuleMatcher.cs | Refactored to use condition matcher factory pattern |
| ConditionMatchers/* | New matcher classes for content, size, date, presence, and name conditions |
| ContentIdentityExtractor.cs | Extracted content identity retrieval logic from matcher |
| Resources files | Added localization strings for access-related errors and UI messages |
| Test files | Comprehensive unit and integration tests with Windows ACL and POSIX permission handling |
| docs/testing-permissions-and-symlinks.md | Detailed documentation for testing permission scenarios |
| CI pipeline | Modified to handle integration tests differently on macOS (no coverage, added diagnostics) |
Files not reviewed (1)
- src/ByteSync.Client/Assets/Resources/Resources.Designer.cs: Language not supported
Comments suppressed due to low confidence (1)
src/ByteSync.Client/ViewModels/Sessions/Comparisons/Results/ContentIdentityViewModel.cs:207
- Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/ByteSync.Client/Models/Comparisons/Result/ContentIdentity.cs
Outdated
Show resolved
Hide resolved
src/ByteSync.Client/Services/Comparisons/ContentIdentityExtractor.cs
Outdated
Show resolved
Hide resolved
tests/ByteSync.Client.UnitTests/Models/Comparisons/ContentIdentityAccessTests.cs
Outdated
Show resolved
Hide resolved
src/ByteSync.Client/Models/Comparisons/Result/ContentIdentity.cs
Outdated
Show resolved
Hide resolved
tests/ByteSync.Client.UnitTests/Services/Inventories/InventoryBuilderInspectorTests.cs
Outdated
Show resolved
Hide resolved
tests/ByteSync.Client.UnitTests/Services/Inventories/InventoryBuilderPublicTests.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
…tor.cs Co-authored-by: Copilot <[email protected]>
…andling' into feature/inventory-inaccessible-handling
|



Context
Inventory can hit access errors (e.g.
UnauthorizedAccessException, protected system folders, POSIX permissions). Previously this could stop the inventory and/or allow rules to propose actions to/from locations that were not actually accessible.What this PR does
IsAccessible(on all FileSystemDescription types).Technical details (summary)
bool IsAccessible { get; set; } = true;onFileSystemDescription.Get*()withEnumerate*()+try/catchonUnauthorizedAccessException,DirectoryNotFoundException,IOException.IsAccessible=false, not registered for analysis; identified volume not increased.ExistsOnconsiders presence even if inaccessible (prevents false "Create" proposals).AtomicActionConsistencyCheckerrejects when source or any target has aFileDescriptionwithIsAccessible=false.ContentIdentity: new computed propertyHasAccessIssue.ContentIdentityViewModel/View: warning icon + access-specific tooltip.Tests
ExistsOnreturns true (presence) while analysis is excluded.finally.chmod 000on the target file; same validation; permissions restored (0644) infinally.User impact
Compatibility
IsAccessible) default to accessible.Notes
Checklist