Skip to content

fix(requests): scope follower notifications per request and recheck server types under lock - #1653

Merged
Quick104 merged 11 commits into
feat/request-revampfrom
fix/request-revamp-kody-review
Sep 29, 2026
Merged

Quick104 merged 11 commits into
feat/request-revampfrom
fix/request-revamp-kody-review

Conversation

@Quick104

@Quick104 Quick104 commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Related issue: #1201
Validation tasks: reaches #1202 C3, #1204 C2, Silo-Server/silo-apple#328 C2, Silo-Server/silo-apple#329 C2, Silo-Server/silo-android#338 C2, Silo-Server/silo-android#339 C2, #1234 C1, #1235 C2, Silo-Server/silo-apple#336 C1 and Silo-Server/silo-android#346 C1 (none run yet)

Two review findings on #1632 hold up against the code.

  • Follower notifications for series with more than one request. A series in the library can have completed requests still waiting for the library scan beside a newer open request for other seasons. Follows belong to the title, so when the completed request's files arrived, its notification also went to the profiles that had followed the newer request, telling them their seasons had arrived when they hadn't, and cleared their follows. They were never told when their seasons did arrive. The reverse also happened: declining the newer request deleted every follow on the title, including those still waiting for the completed request.
  • Route and server saves racing. A route save and a server save each validated the other before their transactions, and under the server's row lock they rechecked only the 4K switch. If two admins saved at the same moment, a movie route could end up pointing at a server that had just been switched to Sonarr, or no longer took movies. Every request it routed would then fail when sent.

Approach

  • A follow belongs to the request that was open when it was made. media_request_follows gains request_id, and its key becomes (account, profile, request), so a profile can follow each of a title's requests. The insert reads the open request under the share lock it already took. A request's fulfilled notification, the clear after it, and a decline or withdrawal all go by that request. The following state reads the follow on the title's open request, and unfollowing a title removes the profile's follows on all its requests.
  • A new request takes over the follows of the title's failed requests, before deleting any failed request its requester is replacing, so a follow still survives its request failing. The follows move with an UPDATE, so an unfollow running at the same moment waits and then removes the moved row.
  • The migration gives each existing follow the title's latest request created before it, which is the request that was open then, if that request could still have been open (active, or completed no earlier than the follow). A completed request that hasn't notified also keeps a follow stamped just after its completion when the title has no request since, which covers a follow that committed while the completion ran. Otherwise that request had closed (failed, or replaced and deleted by its requester), and the follow goes to the title's first request since that still has a notification to send. With none, it stays with the title's failed request (the one it was made for, or else the latest) for the next request to take over, or is dropped.
  • A route save rechecks each destination server's type and media types with the row held FOR SHARE, alongside the 4K check it already did. A server save rechecks the routes sending to it with the row held FOR UPDATE, under Standard as well as Advanced. The service's pre-checks use the same messages.
  • media-requests.md describes both rules.

No API change.

The branch also pins the plugin SDK to v0.19.0. #1649 merged into feat/request-revamp pinned to the SDK pull request's commit, which was squash-merged as v0.19.0 and can no longer be fetched, so every Go job failed to download the module. v0.19.0 has the same contents.

Validation

  • A service test covers the notification going only to the follow made before completion and leaving the later one in place.
  • DB-backed tests cover two completed requests for one title each getting only their own follower, a clear sparing a follow made again since, a decline keeping the other requests' follows, a follow committed while a completion was under way (with the completion's timestamp earlier than the follow's), and a new request taking over the follows of a failed request, with and without replacing it. The first fails with title-wide listing and the last without the takeover.
  • DB-backed tests cover one profile following both an older completed request and a newer open one, and an unfollow that waits on an adoption and still removes the follow. The last fails with a delete-and-insert adoption.
  • The migration's Up and Down ran twice in a rolled-back transaction. Fixtures: a title with an older completed request and a newer open one, followed before and after the newer request was created; a failed request's follow; a follow whose request was deleted, with an older notified request and an open one on the title; a follow whose request was deleted, with only a declined request left; a follow for a failed request whose replacement completed but has not notified; a follow stamped just after its request's completion; the same beside a replacement created after the follow; a follow whose deleted request's replacement also failed; and a follow with no request. The backfill assigned the older request, the newer request, the failed request, the open request, the completed replacement, the completing request, the open replacement and the failed replacement, and dropped the two with no request left to tell them. Down kept one follow per profile and title.
  • DB-backed tests cover a route save after the server became a Sonarr, and a server save (both save paths, under Standard) after a route sending it movies was added. Both fail without the fix.
  • go test passes for internal/requests/... and internal/apiv2 with DB-backed tests enabled. make lint-changed and verify-local-paths are clean.

Risks

  • One migration, which rekeys media_request_follows by request and backfills it. The table is new in the request revamp and not on main. Rolling back returns follows to one per profile and title, keeping the earliest.

Checklist

AI Disclosure

🤖 Generated with Claude Code

Note

Scope follower notifications per request and recheck server types under lock

  • Follows are now recorded per request instead of per title. A profile can follow each request of a title separately, and fulfillment notification (notifyFulfilledPending) notifies and clears only the follows attached to that request. Unfollowing remains title-wide. See follows.go and notify.go.
  • Creating a new request adopts follows attached to failed requests of the same title, collapsing duplicate profiles to the earliest follow and moving rows in place within the same transaction. Declining or withdrawing one request no longer deletes follows belonging to other requests.
  • Server edits now always refuse changes to server kind or supported media types that would make existing routes incompatible, including in Standard mode, not just Advanced tier changes. Route saves (SaveRouteConditional) and integration saves (SaveIntegrationWithDefaults, UpdateIntegrationConditional) recheck this under the server row lock, closing a race where a stale route could be saved after a server changed. See routes_admin.go and routing_mode.go.
  • Risk: schema migration 20260929001953_request_follows_request_key.sql backfills legacy follows by creation time and request lifecycle, and deletes rows with no remaining request to notify; the follow uniqueness key changes from account/profile/title to account/profile/request.

Macroscope summarized ae616e0.

Quick104 and others added 2 commits September 28, 2026 23:32
A series can have a completed request still waiting for the library beside a
newer open request for other seasons. Follows belong to the title, so the
completed request's notification went to, and cleared, follows made for the
open request, and declining the open request deleted follows still waiting
for the completed one.

The notification now goes to the follows made before its request completed,
and a decline keeps the follows a completed, not yet notified request of the
title is waiting to tell.

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

A route save and a server save each validated the other before their
transactions and rechecked only the 4K switch under the server's row lock.
Two admins saving at once could leave a movie route pointing at a server that
had just become a Sonarr, or one that no longer takes movies, and every
request it routed would fail when sent.

Both saves now recheck the server's type and media types with the row locked,
and a server save does so under Standard too.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@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: 1b3d8f03-dd6a-47b6-8e23-a505f6867b09

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.

@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-29T01:57:28.004323Z ae616e0 New commits
ℹ️ 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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2d1103e10f

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread internal/requests/notify.go Outdated
Comment thread internal/requests/notify.go Outdated
@macroscopeapp

macroscopeapp Bot commented Sep 28, 2026

Copy link
Copy Markdown

The Validation tasks: line is incomplete: this change filters request-fulfilled notification recipients, so it also reaches the Requests notification cases on each client and the Notifications event-delivery cases. The follow-registration check in #1202 C2 is unchanged. All candidate results are Not run, so no recorded result is being invalidated or unblocked.

Suggested replacement:

Validation tasks: reaches #1202 C3, #1204 C2, Silo-Server/silo-apple#328 C2, Silo-Server/silo-apple#329 C2, Silo-Server/silo-android#338 C2, Silo-Server/silo-android#339 C2, #1234 C1, #1235 C2, Silo-Server/silo-apple#336 C1, and Silo-Server/silo-android#346 C1 (all not run yet)

Automated check: Macroscope check run agent (gpt-6-luna). No validation was performed.

Posted via Macroscope — v1 validation impact

…mpletion

Two completed requests for one series can both wait for the library, and the
newer can arrive first. Bounding followers only by the request's own
completion time told the newer request's notification about the older
request's followers too, and cleared them.

A title has one open request at a time, so a request's followers are the
follows made after the title's previous request completed and no later than
it did. Clearing them is bounded the same way, so a profile that unfollowed
and followed again during the dispatch keeps its new follow.

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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9e329f4a6d

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread internal/requests/follows.go Outdated
Bounding a request's followers by timestamps broke when a follow committed
while a completion was under way: the completion's timestamp comes from the
start of its transaction, so it could be earlier than the follow's, and the
follow was left out of the request it had read.

A follow now records the request that was open when it was made, and a
request's notification, its follow cleanup and a decline all go by that
request. A new request takes over the follows of a failed request, or of one
its requester replaced, so a follow still survives its request failing.
Existing follows go to the title's open request, or else to its latest
completed request that has not been notified.

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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5bebde23f2

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread internal/requests/follows.go Outdated
Comment thread migrations/sql/20260929001953_request_follows_request_key.sql Outdated
A profile following an older completed request of a series could not follow
a newer request for other seasons: the key was per title, the insert kept the
old row, and the title showed as followed. The backfill also gave every
existing follow to the open request, even ones made for an older request.

Follows are now keyed by account, profile and request, and the "following"
state reads the follow on the title's open request. A new request takes over
the follows of the title's failed requests before any it replaces are
deleted. The backfill gives each follow the title's latest request created
before it, which is the request that was open then.

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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9d29a0a88d

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread migrations/sql/20260929001953_request_follows_request_key.sql Outdated
Comment thread internal/requests/follows.go Outdated
…quests' follows

A new request took over a failed request's follows by deleting and
re-inserting them. An unfollow running at the same moment waited on the
deleted row, could not see the new one, and returned while the profile still
followed the new request. The follows now move with an UPDATE, which the
waiting unfollow re-checks and deletes.

The backfill gave a follow the title's latest request created before it even
when that request was already closed by then, as when the followed request
was deleted by its requester's replacement. It now takes that request only if
it could still have been open, and otherwise the title's open request.

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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1b46795c29

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread migrations/sql/20260929001953_request_follows_request_key.sql
Quick104 and others added 3 commits September 29, 2026 01:15
The download-progress change pinned the SDK to its pull request commit,
which was squash-merged as v0.19.0 and is no longer fetchable, so the Go
jobs could not download the module. v0.19.0 has the same contents.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A follow made for a request that later failed stayed with the failed request
when its replacement had already completed and was still waiting to notify,
since the backfill only looked for an open request. It now gives such a
follow the title's first request since the follow that still has a
notification to send.

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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6862713237

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread migrations/sql/20260929001953_request_follows_request_key.sql Outdated
A completion's timestamp is taken when its transaction begins, so a follow
that committed while it ran can carry a later created_at than the request's
completed_at. The backfill took that as the request having closed before the
follow and moved or dropped the follow. A completed request that has not
notified now keeps such a follow when the title has no request created after
it; a follow whose request was replaced and deleted always has that
replacement after it, so the two cases stay apart.

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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ae723e05fb

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread migrations/sql/20260929001953_request_follows_request_key.sql Outdated
A follow whose request was replaced and deleted, where the replacement has
since failed too, was dropped: the fallback looked only at active requests.
Such a follow now stays with the title's latest failed request, for the next
request to take over.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@Quick104
Quick104 merged commit c7fddde into feat/request-revamp Sep 29, 2026
12 checks passed
@Quick104
Quick104 deleted the fix/request-revamp-kody-review branch September 29, 2026 01:55
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.

1 participant