Skip to content

feat(download): add Remove & Delete Files torrent completion action - #1401

Open
classicluna wants to merge 2 commits into
calibrain:mainfrom
classicluna:feat/torrent-remove-delete-files
Open

classicluna wants to merge 2 commits into
calibrain:mainfrom
classicluna:feat/torrent-remove-delete-files

Conversation

@classicluna

Copy link
Copy Markdown

Closes #1027

Summary

  • add a Remove & Delete Files (remove_and_delete) option to the Torrent Completion Action setting
  • after a successful import, remove the torrent and ask the client to delete its downloaded data (delete_files=True), matching what the usenet "move" flow already does
  • clarify the setting description so it says Remove keeps the downloaded files (raised in the issue comments)
  • regenerate the PROWLARR_TORRENT_ACTION entry in docs/environment-variables.md

Behavior

The deletion runs from post_process_cleanup, the same place as the existing Remove and Change Category actions, so it only happens after output transfer and post-processing have succeeded. At that point every file has already been copied or hardlinked into the library, and transfer size checks have passed. A failed import never removes or deletes anything. The torrent client deletes its own data, so Shelfmark does not delete paths itself and remote path mappings are not involved. Keep, Remove, and Change Category behave as before, and Keep is still the default.

If a request matched a completed torrent that was already in the client, this option deletes that torrent's data too. That is the same scope the existing Remove action already applies to.

Validation

  • make python-checks (ruff check, ruff format, basedpyright on backend and tests, vulture): passed
  • make python-test: 3363 passed, 5 skipped, 9 failed. All 9 are in tests/config/test_entrypoint_permissions.py and happen because macOS /bin/bash 3.2 does not support ${1,,} in entrypoint.sh. They do not touch this change.
  • new tests in tests/prowlarr/test_handler.py:
    • Remove passes delete_files=False and Remove & Delete Files passes delete_files=True
    • a failed import with Remove & Delete Files does not call the client
    • the delete case fails without the handler change
  • manual smoke run: PROWLARR_TORRENT_ACTION=remove_and_delete set through real config loading, with a stub client that deletes its folder. The torrent folder was deleted and the hardlinked library file stayed intact.
  • pre-commit hooks (prek) passed

Copilot AI lite review requested due to automatic review settings September 28, 2026 03:01

Copilot AI 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.

Copilot review overview

🔵 Needs a closer look

Moderate issues remain with client-specific deletion behavior and unchecked removal failures.

Review effort: Lite
Findings: None

What changed in this PR

Adds a remove_and_delete torrent completion action that deletes downloaded data after successful imports.

Changes:

  • Adds the new setting and documentation.
  • Passes delete_files=True during cleanup.
  • Adds tests for deletion and failed imports.
  • Review found unresolved client support and removal-failure handling issues.
File Summary
tests/​prowlarr/​test_handler.py Tests the new cleanup behavior.
shelfmark/​download/​clients/​settings.py Adds the new option and description.
shelfmark/​download/​clients/​base_handler.py Applies conditional deletion; client support and failure-result handling need changes.
docs/​environment-variables.md Documents the new configuration value.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot AI review requested due to automatic review settings September 29, 2026 14:03

Copilot AI 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.

Copilot review overview

🟢 Approval recommended

No unresolved blocking issues remain, and the behavior is covered by focused tests.

Review effort: Lite
Findings: None

@classicluna

Copy link
Copy Markdown
Author

@calibrain I opened this for #1027, where you offered to review a PR. The follow-up commit addresses the client-specific deletion and cleanup-failure concerns raised by the automated review; focused tests pass. No rush given your schedule, but I would appreciate a review when you have time. Thanks!

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.

[Feature] Delete completed torrent downloads

2 participants