Skip to content

fix(requests): keep download server details out of requesters' requests - #1651

Merged
Quick104 merged 1 commit into
feat/request-download-progressfrom
fix/requester-target-details
Sep 29, 2026
Merged

Quick104 merged 1 commit into
feat/request-download-progressfrom
fix/requester-target-details

Conversation

@Quick104

@Quick104 Quick104 commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Closes #1646
Related issue: #1643
Validation tasks: reaches #1202 C1, #1204 C1 and C2, Silo-Server/silo-android#338 C2, Silo-Server/silo-android#339 C2, Silo-Server/silo-apple#328 C2 and Silo-Server/silo-apple#329 C2 (none run yet). The requester's request rows show less; nothing else changes.

Regular users' v2 request responses carry admin details. Every request target includes the download server it went to, the routing rule that sent it there, the server's own ids, and its raw status (such as completed/importBlocked). The request itself includes submission errors, which can name servers and routing rules or carry a plugin's raw error. The Android apps print some of this in the My Requests row, for example Radarr • 1080p • downloading • completed/importBlocked. A requester can't act on any of it, and on a shared server it shows how the admin set up their download servers.

Approach

mediaRequestOf now takes the viewer, and it fills the download server details only for an admin:

  • on each target: integration_id, integration_kind, instance_name, external_id, external_status, route_name, last_error
  • on the request: integration_kind, external_id, external_status, last_error

A requester keeps each target's quality, status and download progress, plus the request's state and outcome_reason. This applies to every v2 endpoint that returns a request: create, list mine, get, cancel, and the admin list and actions. Admins see everything, as before.

The request-level last_error goes with the rest. It is written only by failed submissions, and it can quote routing-rule and server names, raw plugin or transport errors, and instructions meant for the admin ("re-save it in admin"). A requester whose request failed still sees that it failed, and why it was declined or cancelled when it was. On the web, that means the Failed badge with no error line under it.

All of these fields were already optional, so the contract check reports no change, and their descriptions now say admins only. /api/v1 is frozen and still returns them, as docs/architecture/media-requests.md now states.

This stacks on #1649, which adds download to the same mapping.

Validation

  • go build ./..., go vet, gofmt, make lint-changed (0 issues) and make test-go pass.
  • make verify-apiv2-openapi, verify-apiv2-contract (no changes), verify-apiv2-fixtures, verify-apiv2-web-types and the committed-artifact test pass.
  • The web typecheck passes, as do the Requests, admin request and request API tests (69).
  • New handler tests check create, list mine, get and cancel as a member and as an admin. They fail for all four endpoints when the old behavior is forced back. Another test checks that the admin list and actions keep every detail.
  • Clients: the Android models default these fields to empty, and the row drops blank parts, so it reads 1080p • downloading. The Apple apps read only quality, status, error and download. The web requester pages use only the error and outcome_reason. None of them needs a change.

Risks

  • Requesters lose the error text on failed requests. Web shows the Failed badge alone, Apple the state without a reason, and Android drops its red error line.
  • A third-party client that showed instance_name to requesters will see it disappear.
  • The client repos' vendored API fixtures need a refresh (make apiv2-fixtures-sync) after this merges.

Checklist

  • I read and can explain the complete diff.
  • This pull request addresses one concern.

AI Disclosure

  • Harness: T3 Code (Claude Code agent harness)
  • Tool(s): Claude Code, including a subagent; GitHub CLI
  • Model(s): claude-opus-5-5
  • Involvement: Fully AI-generated at the maintainer's request. A subagent wrote the change, and the main agent reviewed the diff.
  • Adversarial review: n/a

🤖 Generated with Claude Code

Note

Hide download-server details from requester responses in v2 media request API

Makes mediaRequestOf in requests.go viewer-aware. Requesters no longer receive server kind, server identity, external identifiers and statuses, routing names, and server-related errors at the request or target level; admins still receive all fields. All handlers (create, get, list, cancel, admin list, admin actions) now pass the acting viewer into serialization.

  • Updates the OpenAPI contract, generated TypeScript schema, and contract fixtures; admins get the extra fields, requesters do not.
  • Adds visibility rules docs in media-requests.md.
  • Adds tests across all operations for both viewer roles: TestRequestDownloadServerDetailsAreForAdmins and TestAdminRequestsCarryDownloadServerDetails.
  • Behavioral Change: contracts/api/v2/fixtures/get_system_info_ok.json contract digest changes; requester-visible target fields (quality, status, download progress) are unchanged.

Macroscope summarized 8b109c8.

The profile-scoped v2 request operations (createRequest, listMyRequests,
getRequest, cancelRequest) gave a requester every target's download
server id, kind and name, the server's own id and raw status, the routing
rule and the target error, plus the request's integration kind, external
fields and submission error. These are admin details. They show how the
admin named their servers and routing rules, and the errors can carry a
plugin's raw text, such as a server URL or a release title.

mediaRequestOf now takes the viewer and fills those fields for an admin
only. A requester keeps the request's state and outcome_reason and each
target's quality, status and download progress. An admin still sees
everything, on the profile-scoped operations and on the admin request
operations. /api/v1 is frozen and unchanged.

The fields were already optional, so the contract diff reports no
change; their descriptions now say admins only.

Refs #1646

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@Quick104 Quick104 added the stacked Base is another open PR; merge in stack order named in the body label Sep 28, 2026
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-28T21:23:14.040422Z 8b109c8 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Sep 28, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 01e14f72-8945-4b53-adf7-70e88c1a572f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Quick104
Quick104 merged commit 8a50d47 into feat/request-download-progress Sep 29, 2026
10 checks passed
@Quick104
Quick104 deleted the fix/requester-target-details branch September 29, 2026 00:58
Quick104 added a commit that referenced this pull request Sep 29, 2026
* feat(requests): show download progress while a request downloads

Request-router plugins can now report how far a target's downloads are
(TargetStatus.progress, declared with request_router.reports_download_progress).
Store it on the target in new download_* columns, and show it on the Requests
page, the title page's request bar and the admin queue: a bar once the size is
known and "Downloading · 43% · about 12 min left", or the phase (waiting,
paused, stalled, importing, import blocked).

The reconcile pass records a download's first progress. A new one-minute
refresh_request_downloads task then refreshes only downloading targets that
have progress, from plugins that declare it. It shares a target-write lock
with reconcile, which waits for it rather than skipping, and runs within a
90-second budget. Progress writes leave the target's updated_at, the request
status and its history alone; progress clears on completion or failure, when
the plugin stops reporting it, or 15 minutes after its server stops answering.
A target's raw external status is kept in step with its phase.

The v2 API adds RequestDownload on request targets, requests and the title
detail's request state, and download_progress_supported on the requests
status capability. Clients poll every 30 seconds while something they show
downloads. TMDB title details are cached for two minutes so title pages
polling a download share one fetch.

The plugin SDK is pinned to the silo-plugin-sdk PR commit until v0.19.0 is
tagged.

Refs #1643

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* fix(requests): hold both reconcile locks on one session and size requests from every live target

The reconcile pass took its own advisory lock and the request target write
lock on two pooled connections, so with database.max_connections at 2 its
first query waited forever for a third. pglock gains Lock.AcquireAlso, which
takes a second key on the session already holding a lock, and Release frees
every key the session holds; the reconcile pass takes both locks through it.
pglock.Acquire, which only this pass used, is gone.

A request's combined download progress skipped a live target that had not
reported progress yet, so a 1080p and 4K request could show 90% from the
1080p copy alone. Such a target now leaves the combined size unknown, as the
docs already said.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* perf(requests): index the targets the download refresh pass looks for

The download refresh pass, and its idle check on every API node, select
downloading targets that have progress once a minute. Without an index on
that predicate each run scanned all of media_request_targets, which grows
with request history. A partial index on (request_id, download_checked_at)
where status = 'downloading' and download_phase is set covers only those few
rows, built concurrently.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* fix(requests): keep download server details out of requesters' requests (#1651)

The profile-scoped v2 request operations (createRequest, listMyRequests,
getRequest, cancelRequest) gave a requester every target's download
server id, kind and name, the server's own id and raw status, the routing
rule and the target error, plus the request's integration kind, external
fields and submission error. These are admin details. They show how the
admin named their servers and routing rules, and the errors can carry a
plugin's raw text, such as a server URL or a release title.

mediaRequestOf now takes the viewer and fills those fields for an admin
only. A requester keeps the request's state and outcome_reason and each
target's quality, status and download progress. An admin still sees
everything, on the profile-scoped operations and on the admin request
operations. /api/v1 is frozen and unchanged.

The fields were already optional, so the contract diff reports no
change; their descriptions now say admins only.

Refs #1646

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stacked Base is another open PR; merge in stack order named in the body

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant