fix(import): skip files already tracked in library

[trevorswanson]

Summary

• skip files already present in book_files during manual folder-import scans
• only new/untracked files reach catalogue matching and the import wizard
• document the default behavior

Motivation

Re-scanning a library folder currently presents already-imported files again. Files whose names no longer match catalogue metadata can appear as unmatched, forcing manual re-selection. Folder import should default to new/unmatched files.

Testing

• go test ./internal/db ./internal/importer ./internal/api via Go 1.26.6 container
• Result: all three packages passed
• Added TestManualImportScan_SkipsAlreadyTrackedFiles

Related: #1292

[codecov]

Codecov Report

❌ Patch coverage is 81.81818% with 38 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/api/manual_import.go	88.70%	10 Missing and 4 partials ⚠️
internal/api/manual_import_cache.go	70.00%	9 Missing and 3 partials ⚠️
internal/db/book_files.go	76.92%	3 Missing and 3 partials ⚠️
internal/db/books.go	0.00%	4 Missing ⚠️
internal/api/samedevice_unix.go	80.00%	1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[trevorswanson]

Addressed the CodeQL go/path-injection review finding in commit 6bb42778.

The manual-import folder is intentionally user-selectable, so removing the query parameter would break the feature. The raw query value is now isolated in resolveImportFolder, which: normalizes it, requires an absolute path, passes it through LibraryRoots.ResolveContained (symlink resolution plus strict configured-root containment), verifies the resolved target is an accessible directory, and returns only that approved resolved path. Scan uses the returned path for enumeration and never performs filesystem operations on the raw query value.

Validation: go test -count=1 ./internal/api ./internal/importer ./internal/db passed via the pinned Go 1.26.6 container, and git diff --check passed.

[trevorswanson]

Honestly it looks like the CodeQL issue was there before

bindery/internal/api/manual_import.go

Line 100 in b323202

if _, err := os.Stat(path); err != nil { //nolint:gosec // #nosec G304 -- path is symlink-resolved and confirmed inside a configured library root; RequireAdmin middleware enforced at route level

So I don't know if I'm going to solve for that one here, CodeQL's doing its job, but I don't think my PR actually made it less secure if the original source was already suppressing the alert with nosec?

[vavallee]

Thanks for chasing this. Quick note on CodeQL: #nosec silences gosec, not CodeQL, so nothing was suppressing that alert. It's open on main today, one of 116, and it isn't a required check. Three of your five are in files you don't touch and have been open since July. Moving the query param into resolveImportFolder moved the taint source onto a changed line and pulled them in. Your fix commit is what took it from 1 to 5. Leave it, the refactor is good.

Two things before merge.

Big folders come back empty. The filter runs after enumerateImportUnits caps at 1000, so if the first 1000 units are all imported you get items: [] with truncated: true. I ran 1100 tracked epubs plus 5 new ones: main gives 1000 items, this branch gives 0. And ImportTab.tsx:366 says "No book files or folders found here." while the truncation warning only renders when items.length > 0, so the user is told the folder is empty. Filter before the cap, or refill after.

Scan cost scales with the library. Line 567 stats every book_files row per scan, plus ListFiles per confident match at 613. strace, 3000 files, 300 candidates: main 3016 newfstatat, yours 6316. 13ms vs 1.5ms locally, but a lot of people are on NFS, and #1473 already bit this handler on the 120s WriteTimeout.

One question. Already imported files are now unreachable, no toggle, no UI change. Corrupt file or a relink can't be surfaced. Prefer a "show already imported" switch to a hard filter?

Rest looks good, tests green here too.

[trevorswanson]

Sorry for the messy commit history -- I merged in the wrong direction on my fork and had to rebase/clean up, I accidentally combined all my PRs into this one, and had a difficult time detangling it. The last 14 commits, all pushed at 9/10/2026 9:18pm EDT, should be the sum of the changes.

I didn't want to lose the conversaiton history here but I'd be happy to cut a clean branch and make a new PR -- or if you're planning to squash, it might be a moot point.

On to your feedback:

Thanks for chasing this. Quick note on CodeQL: #nosec silences gosec, not CodeQL, so nothing was suppressing that alert. It's open on main today, one of 116, and it isn't a required check. Three of your five are in files you don't touch and have been open since July. Moving the query param into resolveImportFolder moved the taint source onto a changed line and pulled them in. Your fix commit is what took it from 1 to 5. Leave it, the refactor is good.

Left my refactor in since you said its' good

Big folders come back empty. The filter runs after enumerateImportUnits caps at 1000, so if the first 1000 units are all imported you get items: [] with truncated: true. I ran 1100 tracked epubs plus 5 new ones: main gives 1000 items, this branch gives 0. And ImportTab.tsx:366 says "No book files or folders found here." while the truncation warning only renders when items.length > 0, so the user is told the folder is empty. Filter before the cap, or refill after.

This should be addressed now, filtering before the cap and refilling after, to ensure we don't truncate prematurely.

Scan cost scales with the library. Line 567 stats every book_files row per scan, plus ListFiles per confident match at 613. strace, 3000 files, 300 candidates: main 3016 newfstatat, yours 6316. 13ms vs 1.5ms locally, but a lot of people are on NFS, and #1473 already bit this handler on the 120s WriteTimeout.

Added caching so that we don't have to stat every file every time. I believe this should improve performance. Unfortunately since we're using that path variable again, CodeQL isn't happy.

One question. Already imported files are now unreachable, no toggle, no UI change. Corrupt file or a relink can't be surfaced. Prefer a "show already imported" switch to a hard filter?

Added a "show already imported" checkbox, matching the formatting/placement of the "Show excluded" box on the wanted page.

[vavallee]

Thanks for the rework, the starvation fix and the toggle both look right and the tests for them are good. Three things before this can go in.

1. The tracked file cache goes stale. It is keyed on BookFileRepo.version, but book_files rows also disappear through the FK cascade when a book or author is deleted, and through UntrackFilePath in calibre rollback, and none of those bump the counter. I reproduced it: seed a tracked epub, scan (hidden), delete the book keeping the file, scan again, still hidden. Either bump the version in those paths too or key the cache on something read from the table itself.

2. The cold rebuild is heavier than the version I measured last time. Same setup, 3000 tracked rows and 300 new files: main does 8 stats per scan, this branch does about 15,600 cold and 609 warm. Cold is roughly 5 stats per tracked row because confirmedCrossDevice restats the root and both paths for every row, and the cache is cold after every import, so scan then import then scan hits it every time. Stat the scan root once per rebuild and compare device ids directly. The warm number is also doubled because isAlreadyTracked runs in roundSkip and again in the results loop for the same unit, and the second call only matters when includeImported is on.

3. Four new i18n keys only exist as inline defaults. Please add manualImport.alreadyImported, manualImport.showImported, settings.import.bulkAlreadyImported and settings.import.bulkShowImported to en.json.

Also six of the commits are authored and signed off as Claude rather than you. I squash on merge so it will not matter for the final commit, just flagging that the DCO trailer on those is not really a certification.

CodeQL alerts on samedevice_unix.go are the same moved taint source thing as before, ignore them.

[vavallee]

Hi @trevorswanson, checking in on this one. The rework fixed the big things and I'd like to get it merged, there are just the three items from my last review left:

1. The tracked file cache not noticing deletions (book or author delete cascades, and the Calibre rollback). Keying it on something read from the table, like the row count plus the highest id, would avoid having to bump the counter everywhere.
2. The cold scan doing about 5 stats per tracked row. Stat the scan root once per rebuild, and skip the second isAlreadyTracked call unless the show imported toggle is on.
3. The four i18n keys into en.json.

Don't worry about the commit history, I squash on merge. The branch is a bit behind main but still merges cleanly, so no rebase needed either.

If you're short on time, say the word and I'm happy to push those three fixes to your branch myself so your work lands with you as the author. Otherwise I'll leave it with you.

[trevorswanson]

Hey sorry, life got in the way. I'll try to get to your feedback across my PRs today

[trevorswanson]

Looks like a lot has changed since I started this PR so I rebased to resolve all the merge conflicts and then went after your feedback.

Rebase:

• ManualImportPage.tsx got split into web/src/pages/import/{FolderImportView,FolderImportRow,folderImport.ts} alongside the new "adopt from library" view, and Settings → Import bulk-scan was replaced with a link out to the Import page. Ported my skip-tracked feature onto the new structure: the useFolderScan hook, "Show already imported" toggle,alreadyImported badge, and disabled-until-selected per-row Import button all now live in FolderImportView/FolderImportRow. resolveImportFolder is kept and now uses outsideRootsMessage for its 403 body, since that landed on main after this branch forked. No upstream behavior was reverted.

PR Feedback:

• 1. Stale cache — Fixed by keying the tracked-file cache on the table itself instead of a counter. BookFileRepo.Fingerprint() reads (COUNT(*), MAX(id)) straight off book_files on every scan; BookFileRepo.Version() (the atomic counter) is gone. Confirmed the repro (seed → scan hides it → delete the book → scan still hid it) and added a regression test (TestManualImportScan_RevealsFileAfterBookDeletedByCascade) covering the FK-cascade path specifically, since that's the one a counter structurally can't see no matter how many call sites get patched.

• 2. Cold rebuild perf — trackedFileIndex now resolves the scan root's device id once per call instead of once per tracked row: confirmedCrossDevice is gone, replaced by reading the device id off the os.FileInfo the loop already stats for each row. That's 1 stat/row now instead of up to 5 (2 root + 3 row, from confirmedCrossDevice re-stating the root and importer.SameDevice re-stating both sides). Also dropped the second isAlreadyTracked call in the results loop for the !includeImported case — roundSkip already filtered those out, so it was always recomputing false.

Re-ran your exact benchmark shape (3000 tracked rows, 300 new files) with two worktrees at the commit before this fix and at HEAD, counting actual newfstatat syscalls via strace -c on the compiled test binary (isolating scan cost from DB-setup noise). Results:

	cold scan	warm scan
before fix (4b656dc)	15,608	608
after fix (HEAD)	3,308	308
reduction	4.7×	2.0×

This doesn't reach main's flat 8-stats-regardless-of-library-size, because main has no already-tracked concept at all — its cost doesn't depend on tracked-row count, ours still does (once per cache rebuild, each tracked row needs one stat to confirm it's still on disk). What this fix removes is the extra ~5× multiplier on top of that, not the O(n) itself.

• 3. i18n — Added manualImport.alreadyImported and manualImport.showImported to en.json. The other two(settings.import.bulkAlreadyImported/bulkShowImported) doesn't exist anymore post-rebase (superseded by the Import page).

Commit sign-off/DCO — I added my DCO to each commit with co-authored by Claude but it's causing the CI to error just like in #2486 -- let me know what you want me to do with it.

Verified: full build (linux/darwin/windows), go vet, golangci-lint (pinned v2.11.4), gofmt, go test ./..., go test -race on the touched packages, and the full frontend suite (tsc, eslint, vitest, vite build) all pass clean.