Bug: manual import with includeCompanionFiles drops every companion file, and reports success

[m4bard]

Summary

A manual import with includeCompanionFiles: true moves the audio and leaves everything that was sitting beside it behind. Not some of them, and not intermittently: every companion, every run. The API reports the import as successful, and the only sign anything went wrong is a warning per file in the server log.

The cause is one argument. The companion pass hands the ownership store the book folder as its managed boundary, and the authorizer requires that boundary to be a configured root folder rather than to be inside one, so it refuses. The audio file for the same book, going into the same directory, selects its boundary a different way and is authorized fine.

What I measured

Against ghcr.io/listenarrs/listenarr:canary at c92e6089 (1.3.1), with a check in a public test-data repo so none of this depends on my setup:

• tools/validate_companion_import.sh

It generates a book, drops book.nfo, book.opf, reader-notes.txt and metadata.json beside the source audio, imports with includeCompanionFiles: true, and then reads the destination folder that the API itself named in its response.

destination contents:            ['The Valley of Fear.m4b']
book.nfo           at destination: absent
book.opf           at destination: absent
reader-notes.txt   at destination: absent
metadata.json      at destination: absent
'Failed to import companion file' log lines:   4
'boundary is not a configured root folder':    4
server's own summary: [INF] Manual import companion-file pass completed with 0 imported companion file(s)

The audio arrived and the import reported success. That is the control that makes the rest readable: the tool refuses a verdict if the audio is missing, because then it is looking in the wrong folder and nothing else it says means anything. A blacklisted decoy.bak is the second control, and it correctly did not arrive, so the inspection is telling files apart rather than reporting everything as absent.

The server's own record of one of the four:

[WRN] [ManualImportCompanionImporter] Failed to import companion file
      /data/src/Arthur Conan Doyle/2006 - The Valley of Fear/book.opf during manual import
System.InvalidOperationException: The requested directory boundary is not a configured root folder.
   at LibraryDirectoryOwnershipBoundaryAuthorizer.AuthorizeAsync(...) in LibraryDirectoryOwnershipBoundaryAuthorizer.cs:line 238
   at EfLibraryDirectoryOwnershipStore.EnsureCreatedHierarchyAsync(...) in EfLibraryDirectoryOwnershipStore.Hierarchy.cs:line 32
   at ManualImportCompanionImporter.ImportAsync(...) in ManualImportCompanionImporter.cs:line 245

One thing I checked rather than assumed, because it is the obvious first guess and it is wrong: the source folder's location has nothing to do with it. The check runs the same import twice, once with the source outside every configured root folder and once with it inside one, and the outcome is identical. Whatever refuses these files is looking at the destination.

Worth saying explicitly, because it changes how bad this is: nothing regenerates the lost files. There is no metadata.json writer anywhere in the tree, and none appeared at the destination in any run. .nfo and .opf carry the finding for the same reason, since Listenarr generates neither, so their absence cannot be explained away by a replacement.

Where it comes from

ManualImportCompanionImporter.ImportAsync computes its boundary from the destination paths the import just produced:

var destinationRoot = ManualImportPathPlanner.DetermineScanPath(results
    .Where(r => r.Success && !string.IsNullOrWhiteSpace(r.DestinationPath))
    .Select(r => r.DestinationPath!)
    .ToList());

DetermineScanPath is FileUtils.GetCommonDirectory, so for the ordinary single-book import that resolves to the book folder. It then goes straight through as the managed boundary:

await _directoryOwnershipStore.EnsureCreatedHierarchyAsync(
    destinationDirectory,
    destinationRoot,
    ...

and LibraryDirectoryOwnershipBoundaryAuthorizer.AuthorizeAsync matches that boundary against the configured roots by equivalence:

var rootMatch = roots
    .Select(candidate => new { Root = candidate, Semantics = TryGetPersistedRootSemantics(candidate) })
    .SingleOrDefault(candidate => candidate.Semantics.HasValue
        && candidate.Semantics.Value.Syntax == semantics.Syntax
        && FileSystemPathIdentity.AreEquivalent(
            candidate.Root.Path, canonicalBoundary, candidate.Semantics.Value));
if (rootMatch == null)
{
    throw new InvalidOperationException(
        "The requested directory boundary is not a configured root folder.");
}

A book folder is inside a root and is not equal to one, so this is refused every time. ImportAsync catches, logs the warning, and carries on to the next companion, which is why the import as a whole still reports success.

The primary audio file never hits this because it picks its boundary differently, in ManualImportController.DirectoryOwnership.cs:

var boundary = LibraryDirectoryOwnershipPlanning.SelectMostSpecificBoundary(
    destinationDirectory,
    rootFolders.Select(root => root.Path),
    semantics);
boundary ??= fallbackBoundary;

SelectMostSpecificBoundary filters the candidates by IsSameOrInside and takes the longest, so given the configured roots it returns the root folder. Same store method, same destination directory, different boundary argument, and only one of the two is a root.

Why I think this is an oversight rather than a deliberate rule

Two things point that way.

Three code paths call EnsureCreatedHierarchyAsync with a managed boundary. The manual-import audio path and RenameService.DirectoryOwnership.cs both select theirs with SelectMostSpecificBoundary over the roots they were given, and they are the only two callers of it in the tree. The companion pass is the third path and the one that does not.

And the tests cannot currently see the difference. All four existing companion tests build the ownership store as a mock and match the boundary with It.IsAny<string>(), so the real authorizer never runs. They pass identically whether the boundary is the book folder or the root, which is a reasonable explanation for how this shipped without anyone noticing.

What I would suggest

Select the companion boundary the way the audio path already does, with SelectMostSpecificBoundary over the configured roots and destinationResolution.BoundaryPath as the fallback, and skip the companion with a warning if neither yields one. ImportAsync takes a rootFolders argument, supplied at its single call site in ManualImportController where rootFolders is already in scope a few lines above.

I picked that over the alternative because it changes nothing about what is allowed. The authorizer keeps its current strictness, and the companion ends up authorized against the same root folder that the audio file it accompanies is already authorized against. Two files, twenty-two lines.

The alternative would be to relax the authorizer so a boundary inside a root is accepted. That is a wider change to an authorization check, it affects the rename and download-import paths as well, and it is your call rather than mine. Say the word and I will redo it that way instead.

I have this on a branch and can open a PR. It adds a test that captures the boundary argument handed to the store and asserts it is the configured root: it fails on canary and passes with the change. The rest of the download and import suite passes unchanged.

Not the same as two nearby issues

#577 is companion files left behind by a rename, which is a different service and a different mechanism. RenameService builds its move set from tracked audio files, so a companion is never in the operation set to begin with. The two look identical from the outside, which is why I am separating them here rather than filing this as a comment there.

#245 is the ebook sidecar feature request. I cite it only to establish that includeCompanionFiles is meant to do something.

I did not find an existing issue for the manual-import companion path.

[m4bard]

Re-ran the reproduction against canary at 1.3.3. Still reproduces, unchanged, in both cases.

Worth flagging because #864 rewrote this area: it touched ManualImportCompanionImporter.cs, added ManualImportCompanionImporter.Publication.cs, and changed ManualImportController.cs along with two of its partials. The companion pass now picks between EnsureCreatedHierarchyAsync and EnsureAdditiveHierarchyAsync on a publication plan, and publication itself goes through a registration lease. None of that moved this. Both branches still hand the ownership store the same boundary as before.

What the run shows, per case:

outside  reproduced   companion-failure log lines: 4, boundary refusals: 4
inside   reproduced   companion-failure log lines: 4, boundary refusals: 4

book.nfo, book.opf and reader-notes.txt are all absent at the destination. The audio arrives. The API reports success, and the server's own summary line says:

[INF] [ManualImportController] Manual import companion-file pass completed with 0 imported companion file(s)

The refusal underneath is the same one:

System.InvalidOperationException: The requested directory boundary is not a configured root folder.
   at LibraryDirectoryOwnershipBoundaryAuthorizer.AuthorizeAsync(...) :238
   at EfLibraryDirectoryOwnershipStore.EnsureCreatedHierarchyAsync(...) Hierarchy.cs:112
   at ManualImportCompanionImporter.ImportAsync(...) ManualImportCompanionImporter.cs:278

Line 278 on current canary is the EnsureCreatedHierarchyAsync call.

Two things the check controls for, so the result is not just an absence:

• the selected audio file itself arrives, so the tool is looking at the right destination and the import did run,
• a blacklisted decoy does not arrive, so "absent" is not the tool's default answer for everything.

It also reports that no metadata.json was generated at the destination at all on this run, so the masking that control exists to catch did not occur. .nfo and .opf carry the finding regardless, since Listenarr generates neither.

The scope conclusion from the original report still holds: identical outcome whether the source folder sits outside every configured root folder or inside one, so where the source lives is not what decides this.

#832 is rebased onto 1.3.3 and mergeable again. The rebase conflicted exactly here, because #864 split this into the two branches above, and both were passing the book folder. The fix now corrects the boundary in both.