fix(fetchart): prevent deletion of configured fallback cover art

[dtrunk90]

Description

When import.delete or import.move is enabled, the assign_art method calls task.prune(candidate.path) unconditionally. This incorrectly deletes the configured fetchart.fallback file. Add explicit check to skip pruning when the candidate path matches the configured fallback.

To Do

• Documentation. (If you've added a new command-line flag, for example, find the appropriate page under docs/ to describe it.) - not required because it's a fixup for 9ddddf4
• Changelog. (Add an entry to docs/changelog.rst to the bottom of one of the lists near the top of the document.)
• Tests. (Very much encouraged but not strictly required.)

[github-actions]

Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry.

[sourcery-ai]

Hey - I've left some high level feedback:

• The direct candidate.path != self.fallback comparison may be fragile if paths differ in representation (e.g., symlinks, relative vs absolute, case differences on case-insensitive filesystems); consider normalizing and/or using os.path.samefile (guarded for existence) to robustly detect the configured fallback file.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The direct `candidate.path != self.fallback` comparison may be fragile if paths differ in representation (e.g., symlinks, relative vs absolute, case differences on case-insensitive filesystems); consider normalizing and/or using `os.path.samefile` (guarded for existence) to robustly detect the configured fallback file.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[github-actions]

Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry.

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• The os.path.samefile(candidate.path, self.fallback) call will raise if either path doesn’t exist (e.g., missing or deleted fallback), so it would be safer to guard this with an existence check or wrap it in a try/except (OSError, FileNotFoundError) and default to False on error.
• The new removal_enabled expression is getting fairly complex; consider extracting the fallback-protection logic into a small helper (e.g., _is_prunable_source(candidate.path)) to make the intent clearer and easier to reason about.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The `os.path.samefile(candidate.path, self.fallback)` call will raise if either path doesn’t exist (e.g., missing or deleted fallback), so it would be safer to guard this with an existence check or wrap it in a `try`/`except (OSError, FileNotFoundError)` and default to `False` on error.
- The new `removal_enabled` expression is getting fairly complex; consider extracting the fallback-protection logic into a small helper (e.g., `_is_prunable_source(candidate.path)`) to make the intent clearer and easier to reason about.

## Individual Comments

### Comment 1
<location> `beetsplug/fetchart.py:1492-1497` </location>
<code_context>
         if task in self.art_candidates:
             candidate = self.art_candidates.pop(task)
-            removal_enabled = self._is_source_file_removal_enabled()
+            removal_enabled = (
+                self._is_source_file_removal_enabled()
+                and candidate.path
+                and (
+                    not self.fallback
+                    or not os.path.samefile(candidate.path, self.fallback)
+                )
+            )
</code_context>

<issue_to_address>
**issue (bug_risk):** Guard `os.path.samefile` against missing or non-existent paths to avoid unexpected exceptions.

`os.path.samefile` may raise `FileNotFoundError`/`OSError` if `candidate.path` or `self.fallback` doesn’t exist, turning a previously skipped removal into an import-time crash. Please either wrap `samefile` in a helper that catches these exceptions and returns `False`, or guard it with an `os.path.exists` check on both paths before calling it.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.33%. Comparing base (30d5157) to head (974d917).
⚠️ Report is 353 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #6283   +/-   ##
=======================================
  Coverage   69.33%   69.33%           
=======================================
  Files         141      141           
  Lines       18788    18793    +5     
  Branches     3061     3061           
=======================================
+ Hits        13026    13031    +5     
  Misses       5117     5117           
  Partials      645      645           

Files with missing lines	Coverage Δ
beetsplug/fetchart.py	74.36% <100.00%> (+0.18%)	⬆️

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry.

[sourcery-ai]

Hey - I've found 1 issue

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `beetsplug/fetchart.py:1454` </location>
<code_context>
+            and self.fallback
+            and os.path.exists(candidate.path)
+            and os.path.exists(self.fallback)
+            and os.path.samefile(candidate.path, self.fallback)
+        )
+
</code_context>

<issue_to_address>
**issue (bug_risk):** Guard `os.path.samefile` call against a possible race between `exists` and `samefile`.

`os.path.exists` doesn’t fully protect against a race where `candidate.path` or `self.fallback` is removed or becomes unavailable between the `exists` calls and `samefile`, which can raise `FileNotFoundError` or `OSError`. Please wrap `os.path.samefile` in a `try/except (OSError, FileNotFoundError)` and return `False` on failure to make this resilient to concurrent file changes or external cleanup.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[dtrunk90]

@snejus Any chance to get this into the next patch release?

[snejus]

Apologies, only noticed your PR now. Just one question here

[snejus]

Note you will want to resolve the conflict in the changelog and move your note under the Unreleased section.

[dtrunk90]

Done and ready to be merged :-)