Skip to content

fix(importer): scan a library folder that is itself a symlink - #3026

Merged
vavallee merged 1 commit into
mainfrom
fix/scan-symlinked-library-root
Oct 6, 2026
Merged

vavallee merged 1 commit into
mainfrom
fix/scan-symlinked-library-root

Conversation

@vavallee

@vavallee vavallee commented Oct 5, 2026

Copy link
Copy Markdown
Owner

Summary

Library Scan never entered a library or audiobook folder that is itself a symlink (/books -> /mnt/storage/books, or a container volume path that is a link). filepath.Walk Lstats its root, sees a link, calls the callback once and stops, so a scan of such a root found no books. Found while reviewing #2961, which documented the limitation in the user guide. This resolves the root before walking and removes that limitation from the docs.

Implementation notes

New helper walkRoot in internal/importer/scanner.go: filepath.EvalSymlinks(root), walk the resolved target, and rewrite every path handed to the callback back under the configured root (filepath.Join(root, Rel(resolved, path))). A root that does not resolve, or resolves to itself, is walked exactly as before.

Stored path decision: the configured root. Imports write book_files under the configured root, the author layout inference and reconciledAudiobookPath strip the configured root, the unmatched unit purge keys on the configured roots, and eligibleUnmatched / the serving containment checks resolve both sides themselves. Reporting resolved paths would have made scan rows disagree with import rows for the same file.

Links inside the root stay unfollowed (deliberate since #2961): only the root is resolved, every entry below it is still Lstat'ed, so a linked author folder is reported as a link and not entered.

Root walkers audited:

Walker Change
scanLibrary walkDir (library + audiobook root) now walkRoot
walkLibraryEntries (LibrarySnapshot / FindExisting on add author) now walkRoot
Manual import scan none, already resolves its folder (resolveImportFolder, #2868) and walks with os.ReadDir
Reorganize, book delete sweeps none, they work from book_files rows, not a root walk
Storage stats none, statfs follows the link
Unit walkers (firstEpubIn, detectUnitFormat, dirSubtreeHasAudio), download walkers none, not library roots
Zip streaming none, goes through os.Root, which opens the root through the link

Checklist

  • Tests added or updated
  • Doc-update gate cleared (docs/User-Guide-Wiki.md, linked folders paragraph)
  • Changelog fragment changelog.d/scan-symlinked-library-root.md

Test plan

  • New tests in internal/importer/scan_symlinked_root_test.go fail on main (with a stub walkRoot = filepath.Walk): scan stores no path, FindExisting returns "", the walk reports only the root link
  • Same tests pass after the fix: scan attaches the book at <configured root>/Andy Weir/Project Hail Mary.epub, a book under a linked author folder inside the root is not attached, FindExisting works for a symlinked library and audiobook root
  • gofmt -l ., go vet ./..., golangci-lint run (v2.11.4, 0 issues)
  • go test ./cmd/... ./internal/... (internal/api hit the default 10m timeout under a load average around 70; rerun alone with -timeout 40m, passed)
  • GOOS=windows go build ./...

🤖 Generated with Claude Code

https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9

filepath.Walk Lstats its root, so a library or audiobook folder configured
as a link (/books -> /mnt/storage/books, or a container volume path that is
a link) was reported once as a link and never entered: Library Scan found
nothing there, and the add author LibrarySnapshot could not spot books
already on disk.

walkRoot resolves the root with EvalSymlinks, walks the target, and hands
the callback every path rewritten under the configured root. Stored paths
therefore stay in the form imports write to book_files and the form the
serving containment checks resolve. Entries inside the root are still
Lstat'ed, so a linked author folder is reported and not entered, as before.

Both root walkers use it: scanLibrary's walkDir and walkLibraryEntries.
Manual import already resolved its folder (#2868), and reorganize, storage
stats and the per unit walkers do not walk a library root.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9
Signed-off-by: vavallee <vavallee@protonmail.com>
@codecov

codecov Bot commented Oct 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@vavallee
vavallee marked this pull request as ready for review October 6, 2026 17:40
@vavallee
vavallee merged commit 669bdd7 into main Oct 6, 2026
43 checks passed
@vavallee
vavallee deleted the fix/scan-symlinked-library-root branch October 6, 2026 17:40

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Post-merge note — PR was merged before this automated review ran.)

Clean change. Correctness, path-rewriting logic, and test coverage all look good. One minor nit for future reference:

scanner.go:3289 — unreachable relErr branch is silently wrong if ever reached

if rel, relErr := filepath.Rel(resolved, path); relErr == nil {
    path = filepath.Join(root, rel)
}

filepath.Rel(resolved, path) cannot fail when path comes from filepath.Walk(resolved, ...) — Walk always emits paths rooted at its argument. The relErr != nil branch is therefore dead code. That's fine in practice, but if it were reached, fn would receive a raw resolved-rooted path, silently breaking the "stored paths stay under the configured root" invariant the PR carefully maintains. A clearer form would either be filepath.Join(root, filepath.Rel(...)) with explicit error propagation or a panic("unreachable") — whichever matches how this codebase handles impossible conditions in internal helpers.

Not a regression, just a note for the next reader.

— 🤖 Bindery triage bot (automated). Reply to correct me; a human will see it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant