qBittorrent answering 200 with the body "Fails." is recorded as a successful grab, for a hash the client never took

[m4bard]

Found while reviewing PR 906, which fixes the neighbouring case and is deliberately scoped away
from this one. Verified by reading canary a630572 and by four unit tests on a branch built from
it. Install observation pending.

What happens

QbittorrentAddWorkflow.AddAsync treats the HTTP status as the whole answer:

• listenarr.infrastructure/DownloadClients/Qbittorrent/QbittorrentAddWorkflow.cs:49
if (!addResponse.IsSuccessStatusCode) is the only test applied to the add response.
• :58 logs "Successfully sent torrent to qBittorrent" as soon as that test passes.
• :93 returns new DownloadClientSubmissionResult(addPlan.Hash, addPlan.Hash), where the hash
  is the one computed locally from the magnet or the torrent file, not anything the client said.
  qBittorrent's /api/v2/torrents/add answers 200 with the body Fails. when it will not take
  the torrent. So a refused add is currently indistinguishable from an accepted one, and the
  caller records a grab:
• listenarr.application/Downloads/Submission/DownloadService.cs:368-374 writes the history
  entry once AddAsync returns without throwing, using the returned identifier.
  The result is a Download row and a grabbed history entry for an info-hash that is not in the
  client. The monitor then polls for it, never finds it, and nothing in the log says the add was
  refused. The user sees a download that is queued and never moves.
  A comment already in the file records half of this. :55-59 on PR 906's branch says that below
  Web API 2.14.0 a duplicate add answers 200 with Fails. rather than 409. The knowledge is
  there; the code does not act on it.

Why this is not the same bug as #906

906 is about a client that answers 409, which is Web API 2.14.0 and above, qBittorrent 5.2.0
and above. It classifies that refusal correctly and leaves the success path alone, which I
think is right for a PR about classification.
This one is the success path, and it is worse in kind rather than in degree. A 409 that is
misclassified still surfaces as a failure to the user. A 200 carrying Fails. surfaces as a
success. And it is not only the duplicate case: the same body is how that build reports a
torrent it will not accept for any reason, so a genuinely bad torrent is also recorded as
grabbed.
I have not measured how many installs are on a client old enough to be in this window, and I
would not want to guess. The behaviour is wrong on any version, because the check is on the
status alone.

Readarr reads the body at both add sites

src/NzbDrone.Core/Download/Clients/QBittorrent/QBittorrentProxyV2.cs:161

// Note: Older qbit versions returned nothing, so we can't do != "Ok." here.
if (result == "Fails.")
{
    throw new DownloadClientException("Download client failed to add torrent by url");
}

and the same at :183 for the file path. The comment is the useful part: a positive test for
Ok. would break against older builds that answer 200 with an empty body, which is why both
Readarr and the branch below test for the refusal rather than for the acceptance.

Reproducing it

On a generated library, with one monitored book a configured indexer can satisfy:

1. Point a qBittorrent download client at a stub that serves the WebUI routes the adapter calls
   and answers /api/v2/torrents/add with 200 and the body Fails..
2. Trigger a search and grab for the book.
3. History shows a grabbed entry and the queue shows a row, for a hash the stub never accepted.
4. The log has "Successfully sent torrent to qBittorrent" and nothing else about the add.
   A real pre-5.2 qBittorrent reproduces step 3 by submitting the same release twice, since the
   second add is refused with the same body.

The branch

fix/qbittorrent-fails-body, one commit (96cdbe62) on top of a630572. It does not depend on 906 and
906 does not depend on it.

• Reads the add response body after the status check and raises
  DownloadClientSubmissionException when the trimmed body is exactly Fails..
• Logs the status and the sanitized body at Error before throwing, so the reason is in the log
  rather than only in the exception.
• Tests for the refusal rather than for the acceptance, for the reason in Readarr's comment.
  Trimming is the one divergence from Readarr, which compares the raw body; a trailing newline
  should not hide a refusal.
• QbittorrentAddWorkflow.cs goes from 96 to 119 lines, well inside the 500-line cap.
  Four tests in tests/Features/Infrastructure/DownloadClients/Qbittorrent/QbittorrentAddFailsBodyTests.cs:
• a 200 carrying Fails. raises a submission failure;
• the same with trailing whitespace still does;
• a 200 carrying Ok. still succeeds and still returns the hash;
• a 200 carrying nothing at all still succeeds, which is the control that stops anyone
  rewriting the check as a positive test for Ok..
  Reverting the production change fails the first two and leaves the last two passing. Full
  backend suite green on the branch.

How this composes with 906

They edit the same method, QbittorrentAddWorkflow.AddAsync, but different blocks: 906's arm
goes inside !IsSuccessStatusCode and this one sits after it, on the success path. The
production file merges cleanly: I checked rather than assumed, and git merge-tree --write-tree
auto-merges QbittorrentAddWorkflow.cs in both directions, so neither order is forced there.
They are semantically independent as well: one covers a refusal that arrives as a status, the
other a refusal that arrives as a body.
Both branches also add to tests/Mocks/Api/QbittorrentApiMock.cs, and there git merge-tree --write-tree reports an actual conflict in both directions: 906 adds AddStatusCode and
AddResponseBody right after InfoStatusCode, and this adds AddSuccessResponseBody at the
same spot, so the two insertions land on the same lines. Whoever lands second gets a real merge
conflict to resolve by hand there, not just a result to double-check.

Open question

Whether the same check belongs on the other qBittorrent calls that gate on status alone.
setCategory and addTrackers both answer 200 in cases where they did nothing, and the second
is already treated as best effort. The add is the one where a false success is recorded as a
grab, so the branch stops there, but the pattern is the same and it is worth deciding
deliberately rather than by omission.
Disclosure: drafted with Claude Code at my direction; I read the cited code at the stated commit and reviewed this before posting.