chore: validate PR 717 review-fix application#751
Closed
therobbiedavis wants to merge 140 commits into
Closed
Conversation
Preserve download-client reported path whitespace for torrent source/content paths while keeping destination validation separate. Add user-provided library destination validation for add, move, and root-folder workflows, allowing filesystem roots for root folders while rejecting parent traversal for concrete destinations.
Allow root-folder configuration to accept filesystem roots, including Windows current-drive roots, while keeping concrete destination paths strict against root-only and parent-traversal targets. Add cross-platform root-folder validation coverage and standardize the new torrent path mapper test header.
Make root-folder operations semantics-aware by threading FileSystemPathSemantics through root path checks, repository migrations, deletion safeguards, and unmatched-file filtering, and block new roots that overlap active relocation boundaries. Improve relocation/move reliability with atomic-rename recovery markers, stricter cleanup reconciliation (including ambiguous quarantine detection), resilient heartbeat/broadcast error handling, and retry/reconcile support for superseded move jobs. Remove the temporary frontend debug test placeholder.
Adds lease-generation fencing for move jobs so only the active claimant can heartbeat, update status/phase, and persist move-manifest cleanup state. Move processing now cancels when a lease is lost, and persistence now increments/stores LeaseGeneration with a migration. Path handling was also made semantics-driven across backend and frontend: remote/client path mapping, scan/planning, metadata parsing, and destination validation now honor explicit syntax/case-sensitivity instead of host defaults or path-shape guesses. Tests were updated to cover lease loss behavior, cross-platform path rules, and new architecture boundaries.
Refactor UpdateJobStatusAsync to isolate persistence, relocation, and broadcast into separate try/catch blocks so persistence failures propagate without broadcasting. Add specific catch blocks in MoveBackgroundService and MoveJobProcessor for MoveLeaseLostException and PersistenceException to prevent lease-lost scenarios from marking jobs as Failed. Update root folder change toast messages for clarity. Add tests covering persistence failure propagation, post-commit relocation failures, and lease-loss handling.
Diagnosis: LibraryMoveWorkflow compares user-library source and destination paths, not internal system paths. The relevant semantics are the destination root/output path boundary semantics already resolved from configured root folders or output path. This changes no public API surface; LibraryMoveWorkflow is DI-constructed and callers continue through LibraryController.
Diagnosis: DownloadImportService plans user-library destination paths under the audiobook destination volume. The correct semantics already flow in through resolved destination semantics. This changes no public API surface; ImportDestinationPlanner now separates planning from committing so a failed file action cannot reserve a batch destination.
Diagnosis: these are user-library path boundaries. Manual import source validation should use source/root semantics, preview/scan containment should use the selected root semantics, and durable content moves already receive resolved semantics from MoveJobProcessor. AudiobookContentMoveRequest is internal-only, so making semantics required does not change public API surface.
Diagnosis: this is enforcement, not a migration of one runtime path. Permanent allow-list entries are internal/runtime helpers; temporary entries are user-library sites that still need explicit semantics. The guard converts future drift into a CI failure while the temporary list is reduced subsystem by subsystem.
Diagnosis: MetadataRescanProcessor compares user-library audiobook and file paths when pruning non-audio file rows. The correct semantics come from the configured root containing the audiobook path, falling back to probing the path boundary. No public API changes are required because the processor already resolves scoped services from DI.
Diagnosis: organize preview and execution compare user-library audiobook paths within a single audiobook boundary. The correct semantics are resolved once from the audiobook current path/root and threaded through ordering, equality, containment, allowed roots, and path summary updates. The public IRenameService API is unchanged; only the service constructor and internal helpers changed.
Diagnosis: AudiobookFileService compares user-library paths while deciding whether a scanned or imported audio file is safe to associate with an audiobook. These are not internal system paths. The correct semantics come from the configured root containing the existing audiobook directory or base path, with boundary probing as fallback. IAudiobookFileService remains unchanged because semantics are resolved inside the service.
Diagnosis: LibraryPathPlanner and FileNamingService compare user-library roots and scan directories, not internal system paths. Scan base-path calculation now receives explicit scan-root semantics, and output-root comparison uses resolved configured-root semantics. IFileNamingService remains source-compatible because semantics resolution is injected optionally and used internally.
Diagnosis: scan queue dedupe compares user-library paths. These are not internal paths, but failures only duplicate or miss queue dedupe rather than mutating files. Active scan and unmatched-scan dedupe now resolve the queued path semantics before comparing. The unmatched last-job cache keeps exact original keys because the controller normally queries the same root-folder path, avoiding a synchronous resolver API change.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Comment on lines
+106
to
+112
| foreach (var root in candidateRoots) | ||
| { | ||
| if (TryValidateResolvedComponents(normalizedTarget, root, out reason)) | ||
| { | ||
| return true; | ||
| } | ||
| } |
Comment on lines
+356
to
+370
| foreach (var directoryEntry in manifest | ||
| .Where(entry => entry.EntryType == MoveJobEntryType.Directory) | ||
| .OrderByDescending(entry => entry.RelativePath.Length)) | ||
| { | ||
| if (FileSystemPathIdentity.TryResolveRelativePathWithinBase( | ||
| quarantineRoot, | ||
| directoryEntry.RelativePath, | ||
| sourceSemantics, | ||
| out var quarantineDirectory) | ||
| && Directory.Exists(quarantineDirectory) | ||
| && !Directory.EnumerateFileSystemEntries(quarantineDirectory).Any()) | ||
| { | ||
| Directory.Delete(quarantineDirectory, false); | ||
| } | ||
| } |
Comment on lines
+59
to
+76
| foreach (var boundary in boundaries) | ||
| { | ||
| if (await ActiveBoundaryConflictsWithTargetAsync( | ||
| path, | ||
| semantics, | ||
| boundary.SourcePath, | ||
| boundary.SourceCaseSensitivityMode, | ||
| cancellationToken) | ||
| || await ActiveBoundaryConflictsWithTargetAsync( | ||
| path, | ||
| semantics, | ||
| boundary.TargetPath, | ||
| boundary.TargetCaseSensitivityMode, | ||
| cancellationToken)) | ||
| { | ||
| return true; | ||
| } | ||
| } |
| return null; | ||
| } | ||
|
|
||
| var anchor = Path.Combine(volumeRoot, firstSegment); |
Comment on lines
+82
to
+94
| if (!string.IsNullOrWhiteSpace(expectedSourcePath)) | ||
| { | ||
| if (string.IsNullOrWhiteSpace(sourceBasePath) | ||
| || !StoredSourcePathMatchesExpected( | ||
| expectedSourcePath, | ||
| sourceBasePath, | ||
| sourceSemantics)) | ||
| { | ||
| throw new ApplicationConflictException( | ||
| "source_path_changed", | ||
| "The audiobook source path changed. Refresh and try again."); | ||
| } | ||
| } |
Comment on lines
+68
to
+79
| if (sourceBoundary != null) | ||
| { | ||
| sourceSemantics = sourceBoundary.Semantics; | ||
| } | ||
| else | ||
| { | ||
| // Metadata-only updates must not require source filesystem access. | ||
| // If the source is not inside a configured boundary, reuse the validated | ||
| // target boundary semantics only for stale-source comparison and best-effort | ||
| // reference rewriting. Invalid source references are preserved by the rewriter. | ||
| sourceSemantics = destination.TargetBoundary.Semantics; | ||
| } |
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.
Temporary same-repository validation branch used to apply and test the four PR #717 review findings. The workflow removes its own helper files before committing the validated production changes. This PR will be closed after the validated commit is transferred to
bugfix/unix-folder-name-space.