Fix weak-storage recovery and root confirmation - #993
Open
therobbiedavis wants to merge 6 commits into
Open
therobbiedavis wants to merge 6 commits into
therobbiedavis wants to merge 6 commits into
Conversation
therobbiedavis
added
the
patch
ChatGPT Codex Connector
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 047681f0c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
It changes core filesystem identity/recovery semantics, adds new persistence structures and migrations, and impacts multiple critical flows (imports, organize, root confirmation), warranting final human verification.
Pull request overview
This PR addresses weak-storage (CIFS/NFS) recovery and root-folder confirmation issues (notably #904) by making filesystem operations capability-first, improving recovery observability, and adding durable rollback / recovery paths for markerless publications and verified organize flows.
Changes:
- Adds structured file-registration recovery blockers/status + a retry API, and wires blockers into root-folder confirmation/relocation gating.
- Hardens weak-storage identity handling (including generic Linux
0x81file handles) to fail-closed for destructive operations while keeping safe read/scan/metadata refresh available. - Introduces batch manifests for compatibility publications and a new verified-rename journal + startup reconciliation phase, with additional backend + frontend UX/tests for warnings and recovery.
File summaries
| File | Description |
|---|---|
| tests/Features/Infrastructure/Persistence/SqliteMigrationSchemaTests.cs | Extends schema assertions for new migration columns/tables. |
| tests/Features/Infrastructure/Persistence/LibraryFilesystemStartupReconciliationServiceTests.cs | Covers new startup reconciliation phase ordering (verified rename). |
| tests/Features/Infrastructure/Persistence/FileRegistrationRecoveryServiceTests.cs | Updates recovery expectations (NeedsAttention / rollback behaviors). |
| tests/Features/Infrastructure/Persistence/FileRegistrationRecoveryProbeTests.cs | Adds probe coverage for Copy/HardlinkCopy + structured blockers. |
| tests/Features/Infrastructure/Persistence/EfAudiobookFileRepositoryBasePathRegistrationTests.cs | Adds coverage for raw physical-generation restore behavior. |
| tests/Features/Infrastructure/Migrations/MigrationMetadataTests.cs | Ensures new EF migrations are discoverable. |
| tests/Features/Infrastructure/Metadata/Jobs/MetadataRescanWeakStorageTests.cs | New regression test for metadata rescan on weak storage. |
| tests/Features/Infrastructure/Library/Scanning/UnmatchedScanBackgroundServiceTests.cs | Updates queue callback to capture warnings. |
| tests/Features/Infrastructure/Library/Scanning/ScanPathAuthorizationServiceTests.cs | Adds legacy-weak identity unsupported test for pinned path-only proof. |
| tests/Features/Infrastructure/FileSystem/RootFolderStorageHealthResolverTests.cs | Adds coverage for legacy weak identity remaining limited. |
| tests/Features/Infrastructure/FileSystem/RootFolderStorageConfirmationServiceTests.cs | Expects structured blocker exception type on confirmation conflicts. |
| tests/Features/Infrastructure/FileSystem/PinnedAudiobookFileRegistrationLeaseTests.cs | Covers OpenForMetadataRead path-only leasing behavior. |
| tests/Features/Infrastructure/FileSystem/FileMutationJournalStoreTests.cs | Adds state-transition guard tests for rollback states. |
| tests/Features/Infrastructure/FileSystem/DockerStorageCapabilityContractTests.cs | Adds native mount identity + remount capability contract tests. |
| tests/Features/Infrastructure/FileSystem/DirectoryObjectIdentityResolverTests.cs | Expands Linux identity classification/weak-evidence coverage. |
| tests/Features/Infrastructure/FileSystem/CompatibilitySourceCleanupCoordinatorTests.cs | Adds manifested-batch behavior coverage for cleanup coordinator. |
| tests/Features/Infrastructure/Downloads/Processing/DownloadProcessingJobProcessorTests.cs | Ensures stable compatibility batch id propagation and retained metadata behavior. |
| tests/Features/Architecture/BackendArchitectureTests.cs | Updates audited “operation namespace” allowlist for new organize prefix. |
| tests/Features/Application/Audiobooks/RootFolders/RootFolderServiceTests.cs | Expects root deletion block message to include operation/state. |
| tests/Features/Api/Services/WorkerProcessorBoundaryTests.cs | Adds Linux-only test for partial unreadable scan paths producing warnings. |
| tests/Features/Api/Features/Library/RootFoldersControllerTests.cs | Adds conflict mapping for identity-unsupported confirmation + warnings passthrough. |
| tests/Features/Api/Features/Library/LibraryController_DeleteFilesystemTests.cs | Adds coverage for legacy-weak identity blocking destructive deletes. |
| tests/Common/PlatformFactAttributes.cs | Adds native mount/remount Fact attributes used by capability contract tests. |
| listenarr.infrastructure/Persistence/VerifiedFileRenameRecoveryService.Probes.cs | New probe helpers for verified-rename batch validation/ownership checks. |
| listenarr.infrastructure/Persistence/Repositories/EfAudiobookFileRepository.PhysicalGeneration.cs | Adds atomic restore APIs and tracked-entry synchronization. |
| listenarr.infrastructure/Persistence/Repositories/EfAudiobookFileRepository.MetadataRefresh.cs | New metadata-only refresh persistence method (no identity writes). |
| listenarr.infrastructure/Persistence/Repositories/EfAudiobookFileRepository.BasePathRegistration.cs | Adds restore-with-basepath transactional path. |
| listenarr.infrastructure/Persistence/Migrations/ListenArrDbContextModelSnapshot.cs | Updates snapshot for new journal/table and manifest columns. |
| listenarr.infrastructure/Persistence/Migrations/20260901142347_AddVerifiedFileRenameJournal.cs | New migration creating VerifiedFileRenameJournals table. |
| listenarr.infrastructure/Persistence/Migrations/20260830025709_AddCompatibilityBatchManifest.cs | New migration adding batch-manifest columns to compatibility journals. |
| listenarr.infrastructure/Persistence/ListenArrDbContext.cs | Adds DbSet for VerifiedFileRenameJournals. |
| listenarr.infrastructure/Persistence/LibraryFilesystemStartupReconciliationService.cs | Runs verified-rename recovery during startup reconciliation. |
| listenarr.infrastructure/Persistence/FileRenameRecoveryProbe.cs | Treats verified-rename journals as rename blockers. |
| listenarr.infrastructure/Persistence/FileRegistrationRecoveryService.RepairState.cs | Uses explicit lifecycle predicates and improves pending messaging. |
| listenarr.infrastructure/Persistence/FileRegistrationRecoveryService.Receipts.cs | Broadens receipt query predicate (publication owner). |
| listenarr.infrastructure/Persistence/FileRegistrationRecoveryService.Protocol.cs | Scopes protocol enforcement optionally to a specific operation id. |
| listenarr.infrastructure/Persistence/FileRegistrationRecoveryProbe.cs | Adds Copy/HardlinkCopy + structured boundary blocker reporting. |
| listenarr.infrastructure/Persistence/Configurations/VerifiedFileRenameJournalConfiguration.cs | New EF entity configuration for verified-rename journal. |
| listenarr.infrastructure/Persistence/Configurations/CompatibilityFilePublicationJournalConfiguration.cs | Adds max length config for new manifest digest column. |
| listenarr.infrastructure/Persistence/CompatibilityFilePublicationRecoveryService.cs | Recovers manifested batches via coordinator at startup. |
| listenarr.infrastructure/Metadata/Jobs/MetadataRescanService.cs | Uses metadata-only refresh when durable generation is unavailable. |
| listenarr.infrastructure/Library/Scanning/UnmatchedScanBackgroundService.cs | Preserves partial results + emits warnings and warningCount. |
| listenarr.infrastructure/Library/Scanning/ScanPathAuthorizationService.cs | Keeps pinned path-only proof for legacy weak identity when still unsupported. |
| listenarr.infrastructure/Library/Moving/RootFolderRelocationService.MetadataRepair.cs | Treats Copy/HardlinkCopy like Move for boundary blocking. |
| listenarr.infrastructure/Library/Moving/RootFolderRelocationService.ExternalRecovery.cs | Includes operation/state in boundary conflict messages. |
| listenarr.infrastructure/Library/Moving/AudiobookFilesystemDeleteService.GenerationProof.cs | Blocks cleanup completion for known-weak identities. |
| listenarr.infrastructure/Library/Moving/AudiobookFilesystemDeleteService.cs | Blocks destructive deletion for known-weak or missing durable identity. |
| listenarr.infrastructure/FileSystem/RootFolderStorageHealthResolver.cs | Treats legacy weak + unsupported current identity as “Limited/Unsupported”. |
| listenarr.infrastructure/FileSystem/RootFolderStorageConfirmationService.cs | Adds structured registration blocker probe + throws dedicated exception. |
| listenarr.infrastructure/FileSystem/PinnedDirectoryCreation.LinuxObjectIdentity.cs | Separates weak generic FID evidence and probes for stronger handle if possible. |
| listenarr.infrastructure/FileSystem/PinnedDirectoryCreation.LinuxIdentityCandidates.cs | Adds durable-vs-weak identity predicates and legacy weak candidate creation. |
| listenarr.infrastructure/FileSystem/PinnedDirectoryCreation.Hierarchy.cs | Exposes legacy weak directory identity candidates for classification. |
| listenarr.infrastructure/FileSystem/PinnedAudiobookFileRegistrationLease.cs | Adds OpenForMetadataRead with safe path-only fallback on weak storage. |
| listenarr.infrastructure/FileSystem/FileMutationJournalStore.AdvanceState.cs | Adds rollback state validation and terminality enforcement. |
| listenarr.infrastructure/FileSystem/FileMover.MarkerlessRegistrationMove.cs | Prevents source retirement for rollback/rolled-back/attention states. |
| listenarr.infrastructure/FileSystem/FileMover.MarkerlessRegistration.cs | Uses lifecycle predicates and supports rollback state handling. |
| listenarr.infrastructure/FileSystem/FileMover.CompatibilityRegistration.cs | Persists compatibility batch manifest fields in claim. |
| listenarr.infrastructure/FileSystem/DirectoryObjectIdentityResolver.cs | Classifies persisted generic FID identities as legacy weak; requires durable candidate for authority. |
| listenarr.infrastructure/FileSystem/CompatibilitySourceCleanupCoordinator.QuarantineCleanup.cs | Extracts quarantine cleanup helper into partial. |
| listenarr.infrastructure/FileSystem/CompatibilitySourceCleanupCoordinator.cs | Adds “all completed” disposition resolution + manifest gating. |
| listenarr.infrastructure/FileSystem/CompatibilitySourceCleanupCoordinator.BatchManifest.cs | New manifest validation/match logic + retain-only completion helper. |
| listenarr.infrastructure/FileSystem/CompatibilityFilePublicationJournalStore.cs | Supports rebinding legacy retained attempts into manifested batches. |
| listenarr.infrastructure/Downloads/Processing/DownloadProcessingJobProcessor.cs | Introduces stable CompatibilityBatchId derivation and always-populated import options. |
| listenarr.infrastructure/DependencyInjection/Library/LibraryRegistrationExtensions.cs | Registers verified rename coordinator + recovery service in DI. |
| listenarr.domain/Downloads/VerifiedFileRenameJournal.cs | New domain journal model and state machine for verified organize. |
| listenarr.domain/Downloads/ImportResult.cs | Adds optional SourcePath to Skipped results + sets disposition accordingly. |
| listenarr.domain/Downloads/FileMutationJournal.cs | Adds rollback states + lifecycle helper predicates. |
| listenarr.domain/Downloads/CompatibilityFilePublicationJournal.cs | Adds persisted manifest columns. |
| listenarr.application/Downloads/Import/DownloadImportService.Quality.cs | Extracts best-existing-quality resolution helper. |
| listenarr.application/Downloads/Import/DownloadImportService.DirectoryOwnership.cs | Plumbs optional batch manifest into publication plan resolution. |
| listenarr.application/Downloads/Import/DownloadImportService.cs | Generates compatibility batch manifest and records SourcePath on skip results. |
| listenarr.application/Downloads/Import/DownloadImportService.Coordination.cs | Uses caller-provided CompatibilityBatchId when available. |
| listenarr.application/Downloads/Contracts/IFilePublicationCapabilityResolver.cs | Extends FilePublicationPlan with manifest fields + helper method. |
| listenarr.application/Downloads/Contracts/IFileMover.cs | Adds rollback API for uncommitted anonymous registration publications. |
| listenarr.application/Downloads/Contracts/IDownloadImportService.cs | Adds CompatibilityBatchId to import options. |
| listenarr.application/Downloads/Contracts/CompatibilityBatchManifest.cs | New manifest struct for expected-member set hashing/validation. |
| listenarr.application/Audiobooks/RootFolders/RootFolderService.cs | Improves delete-block errors with operation/state details. |
| listenarr.application/Audiobooks/Renaming/RenameService.VerifiedExecution.cs | Adds verified organize execution planning and source retirement completion. |
| listenarr.application/Audiobooks/Renaming/RenameService.Rollback.cs | Integrates verified-rename rollback path. |
| listenarr.application/Audiobooks/Renaming/RenameService.Execution.cs | Uses verified transaction coordinator when durable authority is unavailable. |
| listenarr.application/Audiobooks/Renaming/RenameService.cs | Wires verified rename planning/execution into rename flow. |
| listenarr.application/Audiobooks/Renaming/RenameModels.cs | Tracks verified-rename lease on result items. |
| listenarr.application/Audiobooks/Jobs/UnmatchedScanQueueService.cs | Adds warnings to queue job model + update API. |
| listenarr.application/Audiobooks/Files/AudiobookFileService.PhysicalGeneration.cs | Uses snapshot-based restore APIs for rollback preservation. |
| listenarr.application/Audiobooks/Files/AudiobookFileService.MetadataRefresh.cs | New metadata-only refresh operation with strict ownership snapshot checks. |
| listenarr.application/Audiobooks/Contracts/Repositories/IAudiobookFileRepository.cs | Adds metadata refresh + physical generation restore APIs and snapshot types. |
| listenarr.application/Audiobooks/Contracts/PhysicalObjectIdentitySafety.cs | New weak-identity classifier used to block destructive authority. |
| listenarr.application/Audiobooks/Contracts/IVerifiedFileRenameTransactionCoordinator.cs | Adds verified batch manifest model and lease/coordinator interfaces. |
| listenarr.application/Audiobooks/Contracts/IRootFolderStorageConfirmationService.cs | Adds RootFolderRecoveryBlockedException carrying structured blocker info. |
| listenarr.application/Audiobooks/Contracts/IFileRegistrationRecovery.cs | Adds structured blocker/status models + retry API to recovery service contract. |
| listenarr.application/Audiobooks/Contracts/IAudiobookFileService.cs | Adds metadata-only refresh API. |
| listenarr.api/Features/Library/RootFoldersController.Mapping.cs | Adds structured registration recovery conflict payload builder. |
| listenarr.api/Features/Library/RootFoldersController.cs | Maps identity-unsupported + structured recovery blocker exceptions; includes warnings in unmatched results. |
| listenarr.api/Features/Library/LibraryDeleteWorkflow.cs | Treats known-weak identity as unverified delete source. |
| listenarr.api/Features/Library/FileRegistrationRecoveryController.cs | New API endpoint to retry file-registration recovery by operation id. |
| fe/src/views/library/LibraryImportView.vue | Displays scan warnings in Library Import UI. |
| fe/src/views/library/AudiobookDetailView.vue | Refreshes audiobook view after scan completion and improves scan failure toasts. |
| fe/src/types/index.ts | Adds warnings fields to unmatched scan response types. |
| fe/src/stores/libraryImport.ts | Stores and displays scan warnings from API responses. |
| fe/src/components/feedback/UnmatchedFilesModal.vue | Shows warnings; adds polling fallback when SignalR completion is missed. |
| fe/src/tests/UnmatchedFilesModal.spec.ts | Covers cached warnings display and polling-to-completion behavior. |
| fe/src/tests/libraryImport.store.spec.ts | Asserts warnings are persisted into store state. |
| fe/src/tests/AudiobookDetailView.spec.ts | Updates scan completion handling via store-driven updates. |
Review details
Files not reviewed (1)
- listenarr.infrastructure/Persistence/Migrations/20260830025709_AddCompatibilityBatchManifest.Designer.cs: Generated file
- Files reviewed: 120/122 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This was referenced Sep 17, 2026
Open
m4bard
added a commit
to m4bard/Listenarr
that referenced
this pull request
Sep 18, 2026
An independent reviewer went over the two commits before this one. Nothing it found was a
defect in shipped behaviour, but four things were wrong enough to fix before any of this is
offered anywhere, and one of them is a correction to the commit before last.
**The OpenAPI schema moved and nothing said so.** Three representations were checked for
"the stored form does not change" and there is a fourth. BlocklistController declares
ProducesResponseType(typeof(IReadOnlyList<BlockedRelease>)), and Swashbuckle builds a schema
from the CLR shape without reading [JsonConverter], so ReleaseIdentifier was published as an
object with four read-only accessors while the wire carried a string. A spec that contradicts
the wire is worse than either a clean break or no change: a generated client models an object
and receives a string. ListenarrSwaggerRegistration now maps the type, and a test asserts the
schema stays a string and that no schema of its own appears for it. Measured: removing the
map fails that test and nothing else.
**Two Sonarr line citations did not check out.** Both in the NormalizeInfoHash comment, both
inherited from the commit that wrote it. Checked against Sonarr@develop today:
TorrentClientBase.cs:224 is the MonoTorrent call, not :233, which is
EnsureReleaseIsNotBlocklisted; and the call reads MagnetLink.Parse(magnetUrl).InfoHash.ToHex(),
not InfoHashes.V1OrV2.ToHex(), which is the v2 API that Radarr and Readarr use here and Sonarr
does not. TorrentRssParser.cs:85 was right. A maintainer who opened :233 would have found an
unrelated line.
**FromStorage refused an empty key, which is the failure its own comment argued against.**
The comment says a malformed row must not throw on the read path, because the read sits on the
search path and one bad row would take out searching for that book. It then threw on an empty
string, and the column is only NOT NULL, which an empty string satisfies. Measured before the
fix: one row with an empty column made GetForAudiobookAsync throw for that book. It is lenient
now, and the guards that matter are on the write, where Key throws rather than storing a
default and BlockAsync refuses an empty identifier. IsEmpty covers the empty string as well as
the default.
**The JSON converter could not read back what it wrote.** Write went through ToString, so a
default serialised as ""; Read called FromStorage and refused it. No production path
deserialises a BlockedRelease today, which is why it would have been found late. Both halves
agree now, with a test and a control.
**The widened prefix scan added no coverage at all.** The previous commit extended the
inline-literal scan to ReleaseIdentifier.cs and said a bare literal would now be caught there.
ReleaseIdentifier.cs contains no return statement anywhere, every member being
expression-bodied, and the pattern was anchored on `return`. Measured by the reviewer: adding
ForNzbId(id) => new("nzbid:" + id) to that file left all 25 golden-vector tests green. The
pattern no longer anchors on a statement; it matches a literal that ENDS at the colon, which
is what separates a key prefix from "urn:btih:" in the same file. The reflection guard beside
it now reads both types and non-public constants, because prefix application moved and a
private const on the new type was invisible to both guards. Measured both escapes: each now
fails, and the sample in the test carries the must-match and must-not-match shapes.
**Correction to f0e192c.** Its message justifies the change as stopping the branch touching
PR Listenarrs#993's files. That is not true and the reason is worth recording, because it invalidated a
check the whole day was run against. `gh pr view 993 --json files` returns 100 paths; the PR
has 122, GraphQL's files connection caps at 100, and gh prints no warning. The 22 missing paths
were all under tests/Features/Infrastructure/**, and one of them is
SqliteMigrationSchemaTests.cs, which every branch adding a migration must edit because
MigrationHistory_ContainsOnlyRetainedRepairsAndConsolidatedPrMigrationAfterCanary asserts an
exact ordered array of post-canary migration ids. Measured with `gh api --paginate`: the real
overlap is two files, and it was three before f0e192c. Measured with `git merge-tree` against
Listenarrs#993's head: the DbSet line auto-merged, so removing it changed the conflict set not at all.
The single conflict is SqliteMigrationSchemaTests.cs, before and after.
f0e192c stands on its own merits instead, which are real: the entity is registered from its
configuration class, the table name is now explicit and tested, and the file every table-adding
pull request appends to is left alone. No force-push to reword it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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
This PR fixes the weak-storage compatibility and recovery problems behind #904 without treating CIFS/NFS-specific behavior as trusted durable authority.
Weak storage is now handled capability-first: Listenarr keeps read/scan/additive publication available where it can prove those operations safe, keeps destructive mutation fail-closed when durable generation evidence is unavailable, and exposes structured recovery information instead of collapsing root-folder confirmation/path changes into a generic blocked state.
The branch also fixes metadata rescans on weak storage, hardens legacy weak-identity handling, adds durable registration-publication rollback/recovery, and preserves physical-generation state correctly across SQLite compensation paths.
Fixes #904.
Changes
Added
0x81file handles and legacy birth-time-only identities.Changed
Fixed
PlatformNotSupportedExceptionon weak CIFS storage after application restart.DateTimeKind.Unspecified.Testing
Windows/current candidate:
dotnet format --verify-no-changes: clean.git diff --check: clean.Run Tests, CodeQL, and PR version-label validation all pass on exact final head2972b44d41230f5cbcec9adfea7f354bc0611156.Production image — exact final head:
Dockerfilefrom exact final commit2972b44d41230f5cbcec9adfea7f354bc0611156.listenarr-904:2972b44d-final.sha256:4e728f3de32c4104998a82c1cb7df845080a15407a6e8a50f9e8577a604516a7.sha256:a5603b381f24fb6fd4ddd9e01a057c162f64978098458ab627446f79060836a7.sha256:84b929b2ff777989a029c483b48a3ef4ee969246f586c7130b25a7237f383426./audiobookswas mounted from the real reporter-style CIFS fixture and reported filesystem typesmb2inside the exact final image.Real CIFS/SMB validation:
nounix+serverinoCIFS mount.Limited / IdentityUnsupported, not falselyChanged./proc/...pinned reads without enrolling physical identity.Real NFS validation:
Notes
0x81file-handle identity remains compatibility evidence only, never durable destructive authority.