feat(shutdown): [#2169] migrate torrent cleanup to direct token-aware supervision

[josecelano]

Summary

Implements SI-4 of the #1488 shutdown roadmap: torrent cleanup becomes a directly supervised, token-aware top-level component instead of a pre-spawned job in the JobManager legacy registry.

• Replaces torrent_cleanup::start_job() -> JoinHandle<()> with an unspawned run_job(config, torrents_manager, cancellation_token) -> impl Future<Output = Completion> + Send + 'static.
• Removes the job's tokio::signal::ctrl_c() dependency; it now stops on cancellation_token.cancelled() and returns Completion::Cancelled, or Completion::Completed when the weak TorrentsManager reference expires.
• src/app.rs registers torrent_cleanup directly through JobManager::spawn + component_runner; no cleanup JoinHandle, wrapper task, or register_legacy call remains.
• Preserves the configuration gate, immediate first interval tick, cleanup logic, and weak-manager lifetime behavior. JobManager, cleanup policy, peers_inactivity_update (SI-5), and UDP ban cleanup are untouched.
• Adds deterministic lifecycle tests: injected-token cancellation, weak-manager expiry, and the named torrent_cleanup: JobStatus::Cancelled supervisor outcome.
• Adds the reusable manual-torrent-cleanup-e2e skill in a separate commit, as required by the spec.
• Records manual evidence, the independent Task Reviewer report, and issue checkpoints in the issue folder.

Closes #2169

Files touched

• src/bootstrap/jobs/torrent_cleanup.rs
• src/app.rs
• .github/skills/dev/testing/manual-torrent-cleanup-e2e/SKILL.md
• docs/issues/open/2169-1488-si-4-migrate-torrent-cleanup/ (ISSUE.md, verification.md, agent-review-reports.md)

Validation

• cargo test -p torrust-tracker --lib (all root library tests pass, including the three new lifecycle tests)
• Direct-binary SIGTERM run: log shows Stopping torrent cleanup job ... and Job completed after cooperative cancellation job=torrent_cleanup before Torrust tracker successfully shutdown.; no deadline abort for torrent_cleanup.
• Isolated local run with inactive_peer_cleanup_interval = 1, max_peer_timeout = 1, remove_peerless_torrents = false: the announced peer is removed while the torrent remains queryable ("peers": [], HTTP 200). Details in verification.md.
• Independent Task Reviewer: APPROVE, AC1–AC9 pass; its low-severity findings and nits are applied in the last commit.
• linter all, git diff --check, mandatory pre-commit checks, and pre-push checks (nightly fmt/check/doc, full test suite).

The isolated verification used a git-ignored .tmp/ configuration only; no temporary local patches are part of this branch.

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Pull request overview

Migrates the torrent cleanup job to a directly supervised, token-aware component runner aligned with the shutdown roadmap (SI-4), replacing the legacy pre-spawned JoinHandle approach.

Changes:

• Replaces start_job() -> JoinHandle<()> with an unspawned run_job(..., CancellationToken) -> Future<Output = Completion>.
• Wires torrent cleanup into JobManager::spawn using component_runner, removing the legacy registry path.
• Adds lifecycle tests for token cancellation, weak-manager expiry, and supervisor outcome reporting.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/bootstrap/jobs/torrent_cleanup.rs	Reworks cleanup job into an unspawned, token-cancellable runner and adds lifecycle tests.
src/app.rs	Registers torrent cleanup as a direct supervised component and adds a wiring test.
.github/skills/dev/testing/manual-torrent-cleanup-e2e/SKILL.md	Adds a reusable manual E2E verification procedure for torrent cleanup behavior.
docs/issues/open/2169-1488-si-4-migrate-torrent-cleanup/verification.md	Records updated verification evidence for deterministic cancellation and manual runs.
docs/issues/open/2169-1488-si-4-migrate-torrent-cleanup/agent-review-reports.md	Adds an independent reviewer report entry for SI-4.
docs/issues/open/2169-1488-si-4-migrate-torrent-cleanup/ISSUE.md	Updates issue metadata, tasks, checkpoints, and acceptance criteria with evidence.

---

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

[da2ce7]

Review — SI-4 torrent cleanup migration

Reviewed at 9240464ba6334e3468dc09bbc0b99e71d0f915d1 against develop @ e4db63d5, following .github/skills/dev/pr-reviews/review-pr/SKILL.md.

The migration itself is correct and I would approve it on the code alone. Behaviour is preserved path for path, the supervision wiring is exactly what the spec's ownership invariants prescribe, the new tests are genuinely deterministic and assert the named outcome, and all gates are green. One blocking item is documentation-only.

Verification performed

Fresh detached worktree at the head sha, all gates green:

Gate	Exit	Wall
linter all	0	36.3 s (18.0 s at the final head)
cargo clippy --workspace --all-targets --all-features -- -D warnings	0	12.1 s
cargo test -p torrust-tracker --lib	0	24.6 s — 91 passed, 0 failed
cargo test --test lifecycle-signals	0	22.3 s — 6 passed, 0 failed
cargo test --doc -p torrust-tracker	0	0.9 s — 0 doctests
cargo test --workspace --all-targets --all-features	0	145.1 s — 0 failed

The three new tests pass by name, in both the --lib and the full-workspace runs:

app::tests::it_should_register_torrent_cleanup_as_a_direct_cancelled_component ... ok
bootstrap::jobs::torrent_cleanup::tests::it_should_return_cancelled_when_the_token_is_cancelled ... ok
bootstrap::jobs::torrent_cleanup::tests::it_should_return_completed_when_the_torrents_manager_is_dropped ... ok

All four commits are GPG-signed, Conventional Commits subjects, no AI co-author trailers, imports grouped std → external → internal, no added line over max_width = 130.

What I checked rather than assumed

• Config gate (src/app.rs:517) is unchanged context: at inactive_peer_cleanup_interval == 0 the job is not registered at all — no supervisor entry, no Completed outcome.
• Immediate first tick (torrent_cleanup.rs:41) is byte-identical to base, outside the loop, and MissedTickBehavior is unset at both revisions (default Burst). It is not a cancellation blind spot: the first tick() is Ready on first poll.
• Weak-manager lifetime. run_job takes the Arc by value, downgrades it, and the async move block never names the outer binding (the Some(torrents_manager) at line 50 is a fresh shadow), so the strong reference is dropped when run_job returns. This is proved by execution, not only by reading the capture rules: it_should_return_completed_when_the_torrents_manager_is_dropped would time out if the future held a strong Arc, and it passes.
• Cancellation cannot be lost. cancelled() is level-triggered and Interval::tick is cancel-safe, so a cancellation not selected in one iteration is still ready in the next and no tick is consumed. Termination is guaranteed.
• Drop order. src/main.rs:8 binds the container to _app_container, which lives to the end of the match arm — past cancel() (line 13) and wait_for_all (line 15). The weak upgrade therefore cannot start failing mid-shutdown, so the shutdown outcome is Cancelled and never Completed. That matches the recorded SIGTERM evidence and satisfies AC6.
• Legacy census. git grep ctrl_c -- src/bootstrap returns nothing; register_legacy survives only at app.rs:357 (udp_ban_cleanup) and app.rs:531 (peers_inactivity_update), both explicitly out of scope. torrent_cleanup:: has exactly one caller. No wrapper task, no JoinHandle, no dead symbol.
• Skill sample config validated field by field against Core (#[serde(deny_unknown_fields)]) and TrackerPolicy — correct, which matters because a wrong key would abort startup.
• CI at the head sha: five workflow runs registered, all queued/in-progress, no startup_failure.

Blocker

Documentation asserting the pre-migration topology is left unchanged, while ISSUE.md:206 ticks "Documentation is updated when behavior changes".

Three artefacts the repository maintains as current state — not as dated snapshots — become false on merge:

1. docs/features/shutdown-process/task-inventory.md (frontmatter status: verified, last-updated-utc: 2026-09-08): the ownership tree (line 63), the mermaid graph (lines 84, 86), the inventory table row | Torrent cleanup | 0–1 | Legacy registry | Direct Ctrl-C | SI-4 | (line 111), the Legacy Registry prose (lines 162-165, every clause of which is now false), the direct-component count formula and its "three legacy jobs" (lines 176-181), and finding 3 (lines 191-193, "torrent_cleanup and peers_inactivity_update still subscribe to Ctrl-C directly").
2. docs/application-jobs.md:31-32: "Legacy job: one of the three pre-spawned periodic-job handles retained by JobManager outside the JoinSet until SI-4/SI-5 migrate their APIs." Two after this PR, and SI-4 is done.
3. src/bootstrap/jobs/manager.rs:182-184: an in-source instruction to future implementers, // SI-4/SI-5 must migrate these pre-spawned periodic jobs to JobManager::spawn.

I deliberately separated these from the historical record — docs/analysis/20260716-shutdown-process/, docs/issues/closed/**, and the Background sections of #1488/#1588 are dated evidence and should stay exactly as they are.

Why this blocks rather than trailing as a follow-up: task-inventory.md carries an explicit status: verified assertion of currency that this PR silently falsifies; the merge immediately preceding this PR's base is #2168 docs(shutdown): revalidate task inventory, so the project has just demonstrated it treats that document as merge-grade work, and SI-5 will be planned from it; and the acceptance criterion asserting the docs are current is ticked while they are not.

The fix is mechanical and touches no logic: move the row into Direct JoinSet Components, correct the two "three"s and the count formula, rewrite finding 3 to name SI-5 only, update the glossary term, and refresh the manager.rs comment (which is also worth making precise about udp_ban_cleanup, covered by neither SI-4 nor SI-5 — that part is pre-existing).

Suggestions

• biased; in the runner's select! — see the inline comment. Every one of the nine existing token-path select! sites in src/ and packages/ uses it with the cancellation arm first.
• #[instrument(skip_all)] on a now-synchronous constructor — see the inline comment. It no longer spans the job's execution, and it is the only run_* constructor in src/bootstrap/jobs/ that carries the attribute.

Nits

Four minor points inline (skill semantic-link convention, description trigger phrases, a base-vs-tested commit sha in verification.md, and the M3 row's substituted method).

Not raised

run_job taking config: Core and Arc<TorrentsManager> by value diverges from the sibling constructors, which take references — but the merged spec's "Design and Ownership Review" prescribes that exact signature verbatim, so it is settled and spec-conformant.

Happy to re-review promptly once the docs pass lands; nothing else stands between this and an approval.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.92%. Comparing base (0e61ec8) to head (123e49c).

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #2181      +/-   ##
===========================================
+ Coverage    84.86%   84.92%   +0.05%     
===========================================
  Files          351      351              
  Lines        30131    30211      +80     
  Branches     30131    30211      +80     
===========================================
+ Hits         25572    25656      +84     
+ Misses        4186     4184       -2     
+ Partials       373      371       -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[josecelano]

@da2ce7 The branch has been rebased onto the current develop tip (7abc30b2). All threads from review #5152134221 are resolved with their individual responses, and the complete local pre-push gate passed after the rebase. The PR now has no unresolved review threads. Please re-review when the new CI run is complete.

[josecelano]

@da2ce7 Follow-up: CI is now fully green on the rebased head 398cdfef (base develop 7abc30b2), and all review threads remain resolved. The PR is blocked only by the prior CHANGES_REQUESTED review decision. Please submit a re-review/approval when convenient.

[da2ce7]

Round-2 review of SI-4 torrent cleanup migration at 398cdfef8e51ed4871539cedfa9af0195e77f5f1.

Reviewed against develop @ 7abc30b2, following .github/skills/dev/pr-reviews/review-pr/SKILL.md, from scratch over the full scope rather than only the delta.

The code is finished. Everything I raised about the implementation in round 1 is genuinely fixed in the tree, and the two new commits are correct. What still blocks is the same documentation item as round 1 — and the fault for that is partly mine: my round-1 Blocker gave a list of line numbers instead of a swept inventory, so the fix pass corrected exactly the lines I named and left three more sites of the same defect standing. I have now swept exhaustively; the list below is complete, and I will approve on the next push.

Rebase separation

git range-diff of the old series against the new one shows commits 1–4 identical in content and re-hashed only:

1:  9fac6e3d = 1:  cf4d4c30 feat(shutdown): migrate torrent cleanup supervision
2:  7fd68615 = 2:  eafb9f94 docs(skills): add manual torrent cleanup e2e skill
3:  7ae61d3e = 3:  07ad44d0 docs(issues): record SI-4 review and apply review nits
4:  9240464b = 4:  b940585c docs(issues): link PR #2181 to SI-4 spec
-:  -------- > 5:  03a4238c fix(shutdown): address SI-4 review feedback
-:  -------- > 6:  398cdfef test(shutdown): make cleanup expiry assertion deterministic

So all real change is in 03a4238c and 398cdfef, and the branch now sits on the current develop tip (git merge-base develop <head> = 7abc30b2 = develop).

Round-1 items — verified in the tree, not from the replies

Item	Status	Evidence
biased; in the runner select!	Fixed	torrent_cleanup.rs:51, cancellation arm first. Analysis below.
#[instrument] on the sync constructor	Fixed	Attribute and use tracing::instrument; both gone. #[must_use] went with it, correctly: Future is already #[must_use] through the trait, so the attribute was redundant on an impl Future return.
Skill semantic link (skill-link: + ## Skill Links)	Fixed	torrent_cleanup.rs:31 and SKILL.md:170-173.
Skill description trigger phrases	Fixed	SKILL.md:3 now ends with the three phrases.
verification.md evidence commit shas	Fixed	now names 9fac6e3d / 7fd68615 rather than the base.
ISSUE.md M3 Command/Steps self-consistency	Fixed	the row now states the substitution it actually used.
Docs Blocker	Partly fixed — see below

Verified rather than assumed

• biased; order and cancel-safety. Cancellation first is the right order: with a random poll order, a token cancelled in the same poll as a due tick can lose the tie, and the tick arm then runs a full cleanup_torrents() pass charged against the single 10-second deadline in main.rs:15 — the path that turns AC6's Cancelled into Aborted. Both branch futures are cancel-safe: CancellationToken::cancelled() is level-triggered, so a future dropped when the other arm wins resolves immediately when recreated; Interval::tick is documented cancel-safe and consumes no tick when another branch completes first. biased; cannot starve the tick arm either, since the cancellation future is Pending and cheap until cancel. The tick arm's body (cleanup_torrents().await) still runs to completion once selected, so a cancellation arriving mid-pass is observed only at the next loop entry — unchanged from the base and inherent to the shape.

• The new #[allow(...)] is load-bearing, and its reason is true. Tested on a detached worktree at this head by patching only the server copy:

  • remove the whole attribute → clippy denies two errors: needless_pass_by_value at config: Core and manual_async_fn at the signature;
  • keep only manual_async_fn → needless_pass_by_value still denies;
  • keep only needless_pass_by_value → manual_async_fn still denies;
  • swap allow for expect → exits 0, so neither lint is over-broad.

  Clippy's own suggestion for the first is config: &Core, which is exactly what the stated reason rules out: a borrowed parameter cannot produce the 'static future JobManager::spawn requires. Nothing to change.

• Drop soundness off-runtime. If the returned future were dropped without ever being polled, its captured Core, Arc<TorrentsManager> and CancellationToken would drop off-runtime. That is sound: TorrentsManager has no Drop impl and holds only a Core and an Arc<InMemoryTorrentRepository> (itself an Arc<Registry>) — no runtime-affine resource. Moving tokio::time::interval inside the async move block in 03a4238c is a real improvement here, since building an Interval in the synchronous constructor would panic without a reactor.

• drop(torrents_manager) and the Core destructure behave as intended: the strong Arc lives in the future only until first poll, and start_torrent_cleanup hands the future straight to JobManager::spawn, so that is the next scheduler pass.

• Test determinism. All three new tests ran 50 times each from the compiled test binary: 0 failures (3.4 s / 3.5 s / 3.5 s for the fifty runs). See the inline note on the one-poll margin.

• Commits. All six GPG-signed and verified by GitHub, Conventional Commits subjects, no AI co-author trailers, no added .rs line over max_width = 130.

• CI at this head. Five workflow runs, all completed / success, no startup_failure. Docker E2E shows skipping, which is the pre-existing conditional (#2179) and unrelated to this PR.

Gates

Detached worktree at 398cdfef, per-lane target directory, nightly rustc 1.100.0-nightly (a69a63265 2026-09-03).

Gate	Exit	Wall
linter all	0	52.9 s
cargo clippy --workspace --all-targets --all-features -- -D warnings	0	12.1 s
cargo test -p torrust-tracker --lib	0	31.9 s — 91 passed
cargo test -p torrust-tracker --test lifecycle-signals	0	21.6 s — 6 passed
cargo test --doc -p torrust-tracker	0	0.8 s — 0 doctests
cargo test --workspace --all-targets --all-features	0	158.0 s — 0 failed

The three new tests pass by name in both the --lib and full-workspace runs.

Blocker — three more living current-state assertions of the pre-migration topology

Round 1 blocked because documentation the repository maintains as current state would become false on merge. Three of the sites I named are fixed. Three of the same class are not, because I listed line numbers rather than sweeping. Sweeping now (git grep over every non-docs/analysis/, non-docs/issues/closed/ artefact for torrent cleanup together with legacy / Ctrl-C / "three" / SI-4-pending wording), exactly these remain false after merge:

1. docs/features/shutdown-process/task-inventory.md:187 — Findings §1 still reads "The three pre-spawned periodic jobs are a deliberately narrow compatibility registry". Two after this PR. This is in the same document whose last-updated-utc this PR bumps to 2026-09-09 under status: verified, and whose count formula and Legacy Registry section the same commit already corrected — so the document now contradicts itself.
2. docs/application-jobs.md:122-127 ("Current Limitations and Future Work") — "keeps a narrow compatibility registry for the pre-spawned torrent-cleanup, activity-metrics, and UDP ban-cleanup periodic jobs … They are transitional until SI-4/SI-5 migrate their periodic-job APIs." The glossary at :31-33 in the same file was corrected; this paragraph was not.
3. src/AGENTS.md:83-89 — "The torrent-cleanup, activity-metrics, and UDP ban-cleanup jobs retain their pre-existing starter and cancellation semantics: the first two listen for Ctrl-C … register_legacy(name, handle) retains their handles in a narrow compatibility registry … This transitional exception is expected to be removed by the SI-4/SI-5 periodic-job migrations." Every clause of this is false for torrent cleanup after merge. This is the one I most regret missing: it is the architecture contract the SI-5 implementer will read, and the review checklist carries an explicit "AGENTS.md updated if architecture changed" item that is otherwise unmet. (src/AGENTS.md:46 only names the starter in the call tree and stays correct.)

Everything else the sweep surfaced is legitimately untouched and I am not asking for it: the #1488 roadmap row (Open #2169, flipped on close), the #1586/#1588 records and their Background sections, this issue's own folder, and docs/analysis/** / docs/issues/closed/**. That is the complete list; there is no further docs round after this one.

Minor

One inline: the ownership tree line added for torrent cleanup is indented four spaces where every sibling uses six, so the connector does not line up in the rendered block.

Suggestions and nits

Two inline: the one-scheduler-poll margin in the paused-time expiry assertion (with the experiment that measures it), and an optional expect-over-allow note. Plus a nit on the ragged re-wrap at task-inventory.md:180-181 ("The / two legacy jobs are separate and individually / conditional as shown above.") — worth reflowing while the paragraph is being touched for the Blocker anyway.

Not raised

run_job taking config: Core by value clones the whole Core to read one u64, and diverges from the sibling constructors that take references — but the merged spec's "Design and Ownership Review" prescribes that exact signature, the 'static requirement justifies it, and PR #2171 already settled it. docs/issues/open/1588-…/verification.md:38 still shows SI-4 as Pending, but that table's column is "Status of this review" — a dated snapshot of that review, which should stay as it is.

Happy to re-review immediately; nothing but these three doc paragraphs stands between this and an approval.

[josecelano]

@da2ce7 Round-2 feedback is addressed in cb7a1467 and all five threads are resolved. The full pre-push suite passed before publication. This includes the complete current-state documentation sweep, the bounded paused-time scheduler assertion, and the self-checking lint expectations. Please re-review when the new CI run completes.

[josecelano]

@da2ce7 Correction to the prior handoff: the review-fix commit was amended solely to include formatter-required list indentation in , so its final hash is a5fecc82 (replacing cb7a1467). The complete pre-push suite passed again after the amendment, and all resolved-thread fixes remain unchanged.

[josecelano]

@da2ce7 Correction to the prior handoff: the review-fix commit was amended solely to include formatter-required list indentation in task-inventory.md, so its final hash is a5fecc82 (replacing cb7a1467). The complete pre-push suite passed again after the amendment, and all resolved-thread fixes remain unchanged.

[josecelano]

@da2ce7 Final rebase update: the branch is now rebased onto current develop at f6b73e29; PR head is 46f57274. No content changes were made during the rebase. The complete pre-push suite passed on this final history; all review threads remain resolved. Please re-review when the refreshed CI run completes.

[da2ce7]

Reviewed at 46f572749d7c714d8b028e13cc003e05a03ca7a9.

APPROVE. All five round-2 items are addressed in the tree, the three paragraphs that blocked round 2 are now factually true after merge, and the code change that came with them is an improvement I asked for and re-measured.

The three round-2 blocking paragraphs, now discharged

Verified against the tree, not against the reply text.

• docs/features/shutdown-process/task-inventory.md Findings §1 — now separates the historical registry from the current one ("The three pre-spawned periodic jobs were a deliberately narrow compatibility registry. The two remaining pre-spawned periodic jobs are retained in that registry under the same process-wide deadline."). Two is the right number: src/app.rs at this commit has exactly two register_legacy call sites, udp_ban_cleanup (:357) and peers_inactivity_update (:531), and torrent cleanup goes through job_manager.spawn("torrent_cleanup", …) at :518. The file no longer contradicts its own :86, :112, :181 and its Legacy Registry section, which lists exactly those two bullets.
• docs/application-jobs.md:123-128 — the Current Limitations paragraph now names only activity metrics and UDP ban cleanup and states the remaining path as SI-5 plus the UDP ban-cleanup periodic-job migration. It matches the glossary at :31-33 word for word in substance.
• src/AGENTS.md:83-90 — rewritten. "Torrent cleanup is a direct JoinSet component that receives the manager's cancellation token" is true (app.rs:518-525, token from job_manager.new_cancellation_token()); "the former listens for Ctrl-C and the latter keeps its existing manager token" is true for the two survivors (activity_metrics_updater::start_job(config, app_container) takes no token; start_ban_cleanup_job(…, job_manager.new_cancellation_token()) does); register_legacy retaining "those two handles" matches the two call sites; and the removal clause now points at SI-5 and the UDP migration rather than SI-4.

I re-swept the whole tree for this class of statement at this commit — three pre-spawned, three (legacy|pre-spawned|periodic), torrent-cleanup, activity-metrics, SI-4/SI-5, Ctrl-C, and every torrent cleanup line in *.md and *AGENTS.md, excluding docs/analysis/ and docs/issues/closed/. Nothing of the class survives. Everything still matching is a record rather than a current-state claim: this issue's own spec Background, the #1488 roadmap entry, the #1586 and #1588 review records and their verification tables, the docs/copilot-pr-reviews/ logs, and packages/tracker-core/migrations/README.md (three database migrations, unrelated).

The other three items

• Round-2 Minor, tree-diagram indent — fixed. task-inventory.md:49 now carries six leading spaces like every sibling; checked byte-for-byte with cat -A, the │ connectors line up.
• Round-2 Suggestion, expiry-test margin — applied as a bounded yield loop, and I measured what it bought. Patching the bound to 0, 1 and 2 in turn: 0 fails deterministically at the assertion (exit 101), 1 passes, 2 passes. So the assertion still needs exactly one poll and the loop grants eight, with an early break on is_finished() so the extra bound costs nothing when the task is already done. The coupling to tokio::time::advance's internal yield accounting is gone: a tokio release adding a wake step inside advance now has seven polls of headroom instead of zero. 50/50 runs green for each of the three tests (3.40 s / 3.40 s / 3.33 s).
• Round-2 Nit, expect over allow — taken. #[expect(clippy::manual_async_fn, clippy::needless_pass_by_value, reason = …)] compiles clean under -D warnings, so both expectations are still fulfilled and the suppression will now self-invalidate if either lint stops firing.

Verification

Re-run in full after the rebase onto develop at f6b73e29. The rebase is content-free: git range-diff 7abc30b2..a5fecc82 f6b73e29..46f57274 marks all seven commits =, and comparing the two base-to-head patches with blob hashes stripped leaves only three src/app.rs hunk headers whose line numbers shift by the eighteen lines #2178 added above them — not one content line differs. Gates were re-run anyway, because the tree underneath the commits is new.

Detached worktree at 46f57274, clean tree, nightly cargo 1.100.0-nightly (b2e9d5f9d 2026-09-02) / rustc 1.100.0-nightly (a69a63265 2026-09-03):

Step	Exit	Wall
linter all (markdown, lychee, yaml, toml, cspell, clippy, rustfmt, shellcheck)	0	27.7 s
cargo clippy --workspace --all-targets --all-features -- -D warnings	0	7.7 s
cargo test -p torrust-tracker --lib	0	14.5 s — 96 passed
cargo test -p torrust-tracker --test lifecycle-signals	0	20.4 s — 13 passed
expiry / cancel / wiring tests × 20 each	0	1.34 s / 1.38 s / 1.35 s — 0 failures

The lib and lifecycle-signals counts rose from 91 and 6 to 96 and 13, which is the point of re-gating: those are #2178's tests, and they pass alongside this PR's changes on the merged tree. The earlier measurements that are not repeated here — the bound-margin probe showing the expiry assertion needs one poll where the loop grants eight, and the clippy experiments proving both #[expect] entries load-bearing — were made against identical source and stand.

The branch now sits directly on develop at f6b73e29, so the merge is a fast-forward: git merge-tree --write-tree develop refs/quarantine/pr-2181 produced tree 7f19586f, which is exactly the branch head's own tree.

The one optional note I had left — that the two replacement lines in Findings §1 were indented two spaces where their neighbours use three — was taken in the amendment from cb7a1467, whose only change is exactly those two characters of indentation (git diff -w between the two commits is empty).

[da2ce7]

ACK 46f5727 — rebase onto develop f6b73e2 verified content-free (range-diff: all seven commits identical), all three round-2 documentation blockers verified true against src/app.rs, full-tree sweep clean, and the full gate re-run green on the new base: linter all 27.7 s, clippy -D warnings 7.7 s, 96 lib tests, 13 lifecycle-signals tests, 20x3 stress runs with no failures; merges into develop as a fast-forward.

[josecelano]

ACK 123e49c

[josecelano]

Hi @da2ce7 I've rebased, can you ACK the new commit?

[da2ce7]

Re-verified at 123e49cb: the rebase onto 0e61ec8a is content-free (range-diff shows all seven commits identical, and the base-to-head patch is byte-identical to the one approved at 46f57274), and linter all, clippy -D warnings, the torrust-tracker lib tests and lifecycle-signals are green at this head on the build host. The ACK for 123e49cb follows as soon as Testing and Container finish on GitHub, so it is pinned to a green run.

[da2ce7]

Reviewed at 123e49cb9915c1e7a2d8c54e553c968a55079065.

APPROVE, re-affirming my round-3 approval on this head. The rebase onto develop at 0e61ec8a is content-free and I re-ran the verification at this commit rather than inheriting it.

The rebase carries no content

git range-diff f6b73e29..46f57274 0e61ec8a..123e49cb marks all seven commits = — identical content, re-hashed only:

1:  0383bc1e = 1:  f2a82885 feat(shutdown): migrate torrent cleanup supervision
2:  6362c599 = 2:  e70263b7 docs(skills): add manual torrent cleanup e2e skill
3:  d09fce5d = 3:  ef0d5ff5 docs(issues): record SI-4 review and apply review nits
4:  bb34aa15 = 4:  3308f0b9 docs(issues): link PR #2181 to SI-4 spec
5:  9dd21e51 = 5:  5a4c8455 fix(shutdown): address SI-4 review feedback
6:  4a30ebcf = 6:  44c6e5b8 test(shutdown): make cleanup expiry assertion deterministic
7:  46f57274 = 7:  123e49cb docs(shutdown): complete SI-4 ownership updates

Confirmed a second way: the base-to-head patch at this head is byte-identical to the one at 46f57274 once blob-hash index lines are stripped — not even a hunk-header offset differs this time, because the new base commits touch no file this PR touches (git diff --name-only of the two ranges share nothing). Both pinnings report the same 10 files changed, 503 insertions(+), 100 deletions(-).

The base delta f6b73e29..0e61ec8a is the #2186 documentation merge: three files under docs/issues/ and docs/copilot-pr-reviews/, and git diff --stat f6b73e29 0e61ec8a -- src packages tests Cargo.toml Cargo.lock is empty, so no Rust source or manifest moved underneath this branch.

Verification re-run at this head

Detached worktree at 123e49cb, clean tree, nightly cargo 1.100.0-nightly (b2e9d5f9d 2026-09-02) / rustc 1.100.0-nightly (a69a63265 2026-09-03):

Step	Exit	Wall
linter all (markdown, lychee, yaml, toml, cspell, clippy, rustfmt, shellcheck)	0	18.0 s
cargo clippy --workspace --all-targets --all-features -- -D warnings	0	0.7 s
cargo test -p torrust-tracker --lib	0	1.1 s — 96 passed
cargo test -p torrust-tracker --test lifecycle-signals	0	10.8 s — 13 passed

The three tests this PR adds pass by name in the --lib run. The cargo steps are near-instant because the code tree is byte-identical to the one I gated at 46f57274, which is itself the evidence that nothing compiled changed.

The branch fast-forwards onto develop: git merge-base --is-ancestor develop refs/quarantine/pr-2181 is true and git merge-tree --write-tree produced tree b432d277, which is exactly the branch head's own tree — nothing to resolve.

What stands from round 3

Everything, since the source is unchanged: the three documentation paragraphs that blocked round 2 are correct after merge and were checked against src/app.rs (exactly two register_legacy sites, torrent cleanup registered through spawn with the manager's token); the full-tree sweep for pre-migration current-state claims returns nothing; the bounded-yield expiry test was measured to need one scheduler poll where the loop grants eight; and both #[expect] entries are load-bearing with an accurate reason. All 17 review threads are resolved.

One housekeeping note for whoever merges: my round-3 approval was submitted without an explicit commit pin and GitHub attached it to this head, while the ACK comment posted alongside it names 46f57274. The ACK below supersedes it and names this commit.