External resource support for media-server items (Jellyfin / AudiobookShelf) - #1586
Open
GianniCarlo wants to merge 151 commits into
Open
GianniCarlo wants to merge 151 commits into
GianniCarlo wants to merge 151 commits into
Conversation
Closed
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
✅ Claude PR Review —
|
A string literal handed to a SwiftUI API that takes `LocalizedStringKey` already localizes — the non-generic `LocalizedStringKey` overload wins over the generic `S: StringProtocol` one, which is why develop ships bare literals in `Text` and `accessibilityLabel` and they translate fine. The PR had added `.localized` to 34 of them, which reads as though it were required and suggests the bare ones nearby are bugs. Only lines this PR introduced are touched. Develop has plenty of the same redundancy and cleaning it up here would bury the branch's real changes under churn in files it never meant to modify. Everything that genuinely needs `.localized` keeps it: ternaries and `??`, `String(format:)` and `localizedStringWithFormat`, and `String`-typed parameters like `JellyfinLibraryViewModel.navigationTitle`. `files_title` is one of these and is load-bearing — it is a stringsdict plural key in 25 locales, so it has to reach `localizedStringWithFormat` with its count. "Apple Watch (Beta)" goes back to a plain literal. Every other place a tier or product is named to the user is an untranslated brand literal — "BookPlayer Pro" in the settings banner, the profile callout, this same perks list and both login screens — so translating this one alone would have been the odd one out. `apple_watch_beta_title` had no other caller and is removed from Base and en, the only locales that had it. One behaviour difference worth recording: `Text(LocalizedStringKey)` parses the RESOLVED string as Markdown, which `Text(String)` does not. No translation of any of the 33 unwrapped keys contains a Markdown-active character today, so nothing changes now — but a future translation with a `*` or `_` would render as emphasis rather than literally.
`SettingsCompleteAccountView` took a `subType` with a `.pro` default and forwarded it, but no caller ever set it — `SettingsView`, `AccountView` and the preview all construct it bare, so the value was always `.pro`. The parameter read as a variation point that does not exist. The one place that genuinely varies the tier, `ExternalSyncInfoView`, builds `CompleteAccountView(subType: .lite)` directly rather than going through this wrapper, so nothing is lost by fixing the tier here. The `BookPlayerKit` import goes with it: it was there for the `AccessLevel` annotation, and `.pro` infers from `CompleteAccountView`'s own parameter. The file is now one line away from develop.
The file showed 80 changed lines and almost none of it meant anything: the `.lpf` imported-type declaration moved from the top of the dict to the bottom, every alternate app icon was re-sorted alphabetically under both `CFBundleIcons` and `CFBundleIcons~ipad`, `UIPrerenderedIcon` moved below the shortcut colour keys, and the mime-type and filename-extension arrays swapped places. A plist dict is unordered, so the built app is byte-identical either way — it just buried the file in a diff nobody can read. Two real keys were hiding in that noise, and neither should stay. `LSApplicationCategoryType` was added as an empty string, which says nothing. `NSAllowsLocalNetworking` looks like it would help reach a media server on the LAN, but it does the opposite here. develop already sets `NSAllowsArbitraryLoads`, and when a narrower ATS key is present alongside it, iOS 10 and later IGNORE `NSAllowsArbitraryLoads` and honour only the narrow one — this target deploys to iOS 18. So the effect is to shrink plain-HTTP access from "anywhere" down to "local network only", which would break a server reached over a tunnel or a public http address while still working on the LAN. It is also unnecessary for the local case: arbitrary loads already cover it, and the iOS 14 local-network prompt is a different mechanism (`NSLocalNetworkUsageDescription` plus Bonjour) that this key has no bearing on. The file is identical to develop again.
`ExternalSyncIntroView` did not say what it was. It is the sibling of `LoginView` — same benefit rows, same sign-in buttons — pitching the server features and ending on `.lite` where `LoginView` pitches cloud sync and ends on `.pro`. It is now `MediaServersLoginView`, which pairs with the screen it mirrors and matches the user-facing wording of `MediaServersView`, and the file name matches the type again (it was `ExternalSyncInfoView.swift` holding an `ExternalSyncIntroView`, so both names were in circulation for one screen). A doc comment states the relationship, since that is the part the old name hid. The copy from `LoginView` lost three modifiers, and the omission is user-visible. `AppleSignInLink` reads `\.loadingState` from the environment and writes its progress and its errors into it. This screen declared a `LoadingOverlayState` but never injected it and never attached `.errorAlert` or `.loadingOverlay`, so sign-in wrote into the `@Entry` placeholder that nothing observes: no spinner, and a failed Apple sign-in did nothing at all. The same failure on `LoginView` raises an alert. All three are restored. The `ZStack` went with them. `LoginView` needs one because it reserves space with a hardcoded 88pt spacer and overlays the buttons as a sibling; this screen puts the buttons inside `safeAreaInset` as real content, which reserves their height on its own. Once that changed the `ZStack` wrapped a single child. Also: the missing `#Preview`, and two comments that were addressed to whoever was going to finish the screen — "State to trigger your subscription tiers presentation" and "Or whatever your primary button color is". Left alone: the Continue button still carries its own font, padding and background rather than sharing the sign-in buttons' treatment. That is a visual call, not a defect.
The first benefit row pitched "Bring Your Own Server — connect your existing
Jellyfin or Audiobookshelf". Nothing gates that: adding a connection and
downloading from it cost nothing, and only streaming and the progress pull sit
behind the entitlement. The screen is also reachable only from inside a
connected browser, so the row described something the reader had already done,
for free, on a screen asking them to pay. Its two strings go with it.
The sync row claimed progress "stays perfectly synced with your server and
across all your devices", which reads as though the devices talk to each other.
They do not — the cross-device half goes through BookPlayer's servers, and that
is the part the subscription pays for. The disclaimer below already says as
much ("ongoing server costs for cloud storage and progress syncing"), so the
benefit row was the only place not admitting it. It now names both hops.
"Two-way" stays in the title because it is the accurate word for the paid half:
pushing progress TO your own server is free on every tier, and it is the pull
back that needs lite or pro.
The icon follows the copy. `arrow.triangle.2.circlepath` said "sync" but not
"cloud"; `arrow.clockwise.icloud` keeps the sync motion and adds the cloud the
sentence now describes, while staying distinct from the `icloud.and.arrow.up.fill`
that LoginView's own sync row uses. Symbol verified against the system catalog
(available since 2019, against an iOS 18 target) rather than trusted from memory.
The shared disclaimer was written for the cloud funnel and both of its bullets are wrong on this screen. "Due to ongoing server costs for cloud storage and progress syncing" describes storage we do not provide these users: lite sets `.uploadFile: false`, so their audio never reaches our S3 and stays on their own server. Telling a self-hoster they are paying for our storage is both false and the claim least likely to land with that audience. Worse, "you only need an account with us if you plan to listen across different devices" sits directly above Sign in with Apple and Continue with Passkey, and this screen cannot proceed without an account — streaming needs a purchase and the purchase needs the account. The text talked the reader out of the thing the screen was asking for. Deleting the block outright was the other option and I did not take it: the question a self-hoster asks is sharper here than on the cloud screen, not softer — my server, my bandwidth, why am I paying you. Leaving that unanswered on a paywall reads worse than answering it. So the slot stays and the words change: Your audiobook files stay on your own server, never on ours. Your subscription unlocks streaming and covers the cost of syncing your progress between devices through our servers. The two halves are deliberately different verbs. Streaming is UNLOCKED — it costs us nothing, since it is their server and their bandwidth, and pretending otherwise invites exactly the objection above. The syncing is what we COVER THE COST of, because that genuinely runs on our machines. Naming our servers is the point rather than an admission: "we never store your audiobooks" alone implies we store nothing, which is untrue, and a reader who learns otherwise from a network trace has been misled by omission. It says "audiobook FILES" for the same reason. Two things about this branch make the looser wording a claim the code will not back. A media-server book that is DOWNLOADED is not a special object — `SingleFileDownloadService` moves it into the documents folder and the ordinary import pipeline turns it into a regular local `Book`, which `.pro` then uploads. And even on lite the library rows travel: `handleItemsToUpload` schedules `scheduleLibraryItemUploadJob` and `scheduleExternalResourceUpload` for every item, which is what DB-only sync means. The audio is what never leaves their server, so the audio is what the sentence promises. `LoginDisclaimerSectionView` takes its bullets as a parameter now, defaulting to the existing two keys, so `LoginView` is untouched. `ForEach` iterates `.indices` because `LocalizedStringKey` is Equatable but not Hashable, so `id: \.self` over the elements does not compile.
The Stream button wore `arrow.down.circle.dotted` — a downward arrow into a dotted circle, which is iOS's pending-download idiom — while sitting beside the actual download button wearing `square.and.arrow.down`. Two download arrows, and the reader left to work out which one streams. It is `waveform` now, in all three places it appeared: the details screen and both the Label and the Image on the library screen, so the two screens no longer disagree about what streaming looks like. That glyph is now absent from the app. The library tab's "Import files" menu had the opposite problem: it used `waveform`, which everywhere else in this codebase means "this is an audiobook" — the library rows, the Jellyfin and AudiobookShelf item icons, the share extension. Importing is not audio, and the clash got worse once `waveform` also started meaning "stream", so it takes `square.and.arrow.down`. Stream now leads and Download follows, and Stream is driven by the app's shared `PrimaryButtonStyle` rather than a fourth hand-rolled treatment — the feature already had three, differing in height, corner radius and font. The style is passed `theme.linkColor` and white, so the button looks as it did while its geometry, pressed and disabled states come from one place. Download is resized to match the style's 48pt height with the same radius; at its old 68x56 it would have stood 8pt taller than the button beside it. Also folded in, per the request: the redundant `.localized` calls that the earlier sweep missed. That sweep parsed `+++ b/<path>` from the diff, and git appends a TAB to paths containing spaces, so `Path(f).exists()` was false and every file under `Library Screen/`, `ItemDetails Section/` and their siblings was skipped in silence — the media-server feature almost in its entirety. 20 lines across 5 files, all of them PR-added; develop's own redundancy is left alone as before.
The app had four primary-button treatments and no two agreed: the shared `PrimaryButtonStyle` at 48pt with a 24 radius, `IntegrationFlowPrimaryButton` at 14pt vertical padding with a 12 radius, a hand-rolled Stream button at 56pt with a 10 radius, and another hand-rolled one on the media-server paywall at a 12 radius. Three of them existed because the shared one was wearing the wrong clothes. `PrimaryButtonStyle` inverted black and white by theme, which is Sign in with Apple's treatment — and imitating it was pointless, because that button is Apple's own `SignInWithAppleButton` and takes its look from `.signInWithAppleButtonStyle`, never from us. The lookalike mostly appeared on sign-in screens standing next to the real thing. It is the accent treatment now, matching the Pro callout's Learn More button, so the two hand-rolled copies had no reason to exist: `IntegrationFlowPrimaryButton` is deleted and its three call sites plus the paywall's Continue button use `PrimaryButton`. `disabledWithOpacity` is not needed either — the style already dims through `@Environment(\.isEnabled)`. Shapes follow one rule. Everything is a `Capsule`, including Apple's button, whose corners were never a house choice — we simply never styled them. `Capsule` rather than a radius constant because it tracks the height instead of going quietly wrong when someone changes it, and because `cornerRadius(_:)` is deprecated and was only a clip mask anyway, so it was never setting Apple's `cornerRadius` property regardless. The label goes to `.headline` (17pt semibold). At `.title` (16pt callout) it looked undersized in a 48pt button beside Sign in with Apple, whose text scales with its height; `.titleLarge` (20pt) was tried and reads too loud for a one-word label. On the details screen both buttons now take that style — Stream in the accent, Download in the secondary colours — so they cannot drift apart in height, radius or font the way hand-matched geometry does, and they split the row evenly. Download shows its label instead of a bare glyph, reusing the `download_title` string it was already announcing to VoiceOver; the explicit `accessibilityLabel` goes, since a visible `Label` supplies it. It is no longer `SmallDownloadButton`, because it is not small. Download leads and Stream follows, which is also the order develop had.
Comparing it against the `ImportViewController` it will eventually replace, the differences came from three separate causes rather than one drift. The structural one: it re-implemented a navigation bar. The UIKit screen uses a real `UINavigationBar` with `prefersLargeTitles` and `systemItem="cancel"` / `"done"`, which is why iOS 26 gives it a large title and circular glass buttons for nothing. This built an HStack of two buttons with hand-drawn `Circle()`s inside a `ZStack`, so it could inherit none of that. It is a `NavigationStack` with `.toolbar` and `.navigationTitle` now, over a `List` with the house `applyListStyle` — which also brings `toolbarColorScheme`, a modifier that only does anything once a real navigation bar exists. The two fixes were one fix. Two colours were drawn with the background token: the button circles at `systemBackgroundColor.opacity(0.3)` and the row dividers at `0.2`. On a dark theme that is near-black on near-black, which is why the circles were invisible. Both elements are gone — `List` and the toolbar supply them — along with the hardcoded 24pt horizontal padding, so the insets are the system's like every other list in the app. The row icon was a literal `.pink`, ignoring the theme; `ImportTableViewCell` tints it `theme.linkColor`. Rows are sorted with `localizedStandardCompare`, matching `ImportFileItem`'s `Comparable` and the library's own sort — "002" before "010", which plain string ordering gets wrong on the numbered filenames audiobooks nearly always have. Sorted in the shell so every adopter inherits it. The header gains `.localizedCapitalized`, as UIKit has. `import_warning_description` is now a parameter rather than fixed copy, and the external flow passes none: it warns that files are still transferring and the count may change, and neither is true there — the batch is frozen when the sheet opens. Not defaulted, because a default is how that copy reached this screen in the first place. Where it IS supplied it is pinned with `safeAreaInset` in a new `glassCard` helper: as a section footer, a long list scrolled the one caveat you are meant to read before confirming clean off the screen. `glassCard` gives glass an explicit rounded rect, because `liquidGlassBackground` is shaped for floating pills — the mini player, the player's bubble buttons — and stretching it full width produced a capsule with its ends clipped to the display. Swipe-to-dismiss is disabled, deliberately stricter than UIKit, which allows it and runs `try? discardImportOperation()` on the way out — cleanup whose failure it then swallows. The four things the file flow must bring before it can adopt this are documented on the type: closures that can report failure, folder icons with sub-counts, the directory watching that keeps the count live, and counting files inside folders rather than rows. None is reachable from the external flow, whose batch is frozen and whose removals cannot fail.
`LoginView` reserved a hardcoded 88pt of bottom safe area with a clear spacer and then floated the button stack over it as a `ZStack` sibling. The stack is nearer 100pt before Dynamic Type touches it, so the reservation was short from the start: the last line of the disclaimer could not be scrolled clear of the buttons on a short screen. The buttons ARE the `safeAreaInset` now, which is what `MediaServersLoginView` already did, so the list reserves exactly their height whatever that height turns out to be and the `ZStack` goes with it. The opaque background matters as much as the geometry, and both screens needed it. `AppleSignInLink` is an opaque pill and hides whatever passes behind it, which is why this never looked broken in a screenshot — but `ContinueWithPasskeyButton` is bare text over nothing, so the disclaimer scrolled straight through it.
`hasStreamingEnabled()` asked only whether the user had ever paid, and
`donationMade` deliberately outlives logout. A past tipper who signed out kept
media-server streaming with no account at all.
The gate takes `isSignedIn` now, and reads it from `getAccountId()` rather than
`hasAccount()`. Logout blanks the account's fields but leaves the row behind, so
`hasAccount()` stays true afterwards — `AccountServiceTests` asserts it is still
true even after `deleteAccount()`. Only the id going empty marks a signed-out
user.
The separate `donationMade` clause goes at the same time, because it was never
buying anything. It was read directly to dodge `hasPlusAccess()` returning early
on a refunded pro/lite, but RevenueCat attaches `plus` to non-subscription
purchases and that check short-circuits before the refund handling, so
`hasPlusAccess()` already answers for tippers. Two clauses, both load-bearing:
an active tier or a tip, and a subscription that expired without being refunded.
`AccountServiceMock.getAccountId()` mirrors the real one in mapping an empty id
to nil. Without that, a mock account in the logged-out state — blank id, row
still present — reports `Optional("")` and reads as signed IN, which is exactly
the state this gate exists to catch.
`jellyfin-icon` and `audiobookshelf-icon` were image sets, and an image set interpolated into a `Text` draws at its intrinsic point size with its bottom edge pinned to the baseline — font metrics apply to symbol images only. `jellyfin.svg` declared 20x18.3 and `abs.svg` 16x16, against a `.caption` whose cap height is about 8.5pt. So the marks towered over the line they sat in, disagreed with each other, and ignored Dynamic Type entirely, which is the opposite of what the comment above `subtitle` claimed they did. Both are `.symbolset`s now, authored as v4.0 templates against the real guides: each logo scaled so its ink spans exactly baseline to capline, 70.459 units, which is SF Pro's cap height at the 100 points the template is typeset at. Equal heights rather than equal boxes — Jellyfin's mark is square and Audiobookshelf's is 0.84 as wide as it is tall, so matching width would have made one of them look oversized. Measured against Apple's own symbols, flat shapes land at about 1.0x cap height (square.fill 1.015, book.fill 1.022) while round and pointed ones overshoot (triangle.fill 1.043, circle.fill 1.128). These sit at a strict 1.000; the overshoot is a knob to turn if Jellyfin's apex reads small on device. The same artwork fills all three weight slots on purpose: a brand mark has no business getting bolder next to a bold font. With the mark finally the size of the text, the name beside it was a second copy of the same fact, and the expensive one — "Audiobookshelf • " crowded the author off the single line this row gives it, worst at large Dynamic Type where that line truncates first. Glyph and author now, no name and no separator. That leaves the glyph as the only trace of where an item came from, so VoiceOver needs it, and it never had it: `getAccessibilityLabel` builds from title, details, percent and duration, and the row is `children: .ignore`, so the visible name never reached the accessibility tree either. Only the sighted reader ever knew. `includeSource` adds the provider names, leading the announcement the way the glyphs lead the line, and defaults to false so the Watch cell — which draws no glyph — says exactly what it said before. The names are deliberately unlocalized. They are brands, and wrapping them in a `"from %@"` key would be worse than bare: `localized` is a plain `NSLocalizedString` with no fallback to Base, so the two dozen locales without the key would announce the key itself and swallow the name — a regression aimed squarely at the users the change is for. Two things turned up on the way. `DynamicAccessibilityLabelModifier` rebuilds `SimpleLibraryItem` on every progress tick and when the title-source toggle flips, and neither rebuild passed `externalResources`, which the initialiser defaults to nil; the source would have been announced once and then silently dropped. And the order was undefined — `resourcesArray` is `allObjects` off an `NSSet`, so an item linked to two servers could draw its glyphs, and now speak their names, in a different order each launch. `streamingResource` already documents and pins that hazard, so `displayOrderedMediaServerResources` reuses its comparator for everything the user sees or hears. It mattered little while the names were spelled out; it matters now that the silhouette is all there is.
`ProviderName` answers two questions at once: which row is this in Core Data, and which server does it belong to. Hardcover is a provider but not a server — no host, nothing streams from it — so every piece of code that only makes sense for a server carried a case it had no answer for. Three of them answered with `default:`. `ExternalStreamResolver`, `IntegrationHostResolver` and `ExternalUpdateProgressOperation` each handled `.jellyfin` and `.audiobookshelf` and swept the rest into a catch-all. That is right for Hardcover and for a providerName this build doesn't know, and silently wrong for the next media server anyone adds: it would not stream, would not resolve its host, and would not push progress, with no compile error at any of the three to say so. The fourth answered with a placeholder — the library-row glyph returned `plusImageAppIcons`, a 2023 Plus promo PNG, for Hardcover. That arm was already unreachable (the row filters to media servers), and a plain image set interpolated into a `Text` draws at its intrinsic size anyway, which is the bug the comment above `subtitle` warns about. The asset had no other reference in the project and is deleted. `MediaServerProvider` is that second question as a type. `ProviderName.mediaServer` maps onto it in the one exhaustive switch, `isMediaServer` and `SimpleExternalResource.mediaServer` derive from it, and the four sites above now take a value with two cases and nothing to leave unhandled. Adding a media server is a compile error in each of them. It is deliberately not `RawRepresentable`. `ProviderName` owns the strings that reach Core Data, so there is no second spelling to drift from them: `mediaServerRawValues` still derives from `allCases` and the `IN` predicate in `findMediaServerResources(at:)` is byte-identical. Nothing changes for the user — the `case nil:` arms cover exactly what `default:` covered, and `BookView`'s `?? .jellyfin` fallback was already dead behind the media-server filter. `testProviderNameMapsOntoItsOwnMediaServer` pins the `.audiobookshelf` arm, which nothing else did. The existing host-resolver test catches a wrong `.jellyfin` mapping, but no test drives an AudiobookShelf resource through one, so a transposition there compiles, leaves the SQL predicate intact — it filters on nil-ness, not identity — and passes the whole suite while routing every AudiobookShelf item at the Jellyfin connection. Verified by mutating the mapping: it is the only test in the suite that fails. `ExternalProgressService` keeps its `[ProviderName: ExternalProgressProviding]` registry on purpose. It holds only the two servers today, but Hardcover is a progress provider by nature, so that key stays the wider type.
Deleting the Hardcover glyph's placeholder left `plus/` holding four more imagesets nothing asks for: `BookPlayerPlus`, `plusImageSupport`, `plusImageThemes` and `support-no-padding`, all from the 2023 Plus screen that no longer exists. They compiled into every build of the app, the watch app and both frameworks — the catalog is a folder reference, so each is carried by all four targets that list it. Verified unreferenced rather than assumed, because this app force-unwraps `UIImage(named:)` in places and a missed lookup is a crash, not a blank frame. Checked every file type (Swift, storyboards, xibs, Info.plist's 32 `CFBundleIconFiles`, `Icons.json`'s 15 `imageName`s, strings, pbxproj, catalog JSON), case-insensitively and in the generated camelCase symbol form (`.bookPlayerPlus`, `.supportNoPadding`) that a literal grep for the directory name would miss. Also traced every non-literal `UIImage(named:)` argument in the repo — CarPlay's chevron ternaries, `AppIconView`'s `Icons.json` lookup, and `addLayerMask`'s parameter — and none can construct these names. No string interpolation reaches an image lookup anywhere. `plus/confetti/` stays. Four of its five are live in `ConfettiView`, and `confetti_heart` is unreferenced but belongs to that set rather than to the dead Plus screen. Neither `plus/` nor `plus/confetti/` provides a namespace, so those lookups are flat and independent of what sits beside them.
The artwork's corner is a three-state download indicator — a spinner while downloading, nothing once downloaded, a `cloud` in a triangle while the file isn't on the device. That last glyph is right for our own storage and wrong for a media server: the file is sitting on the user's machine, not ours, which is the distinction the media-server paywall copy goes out of its way to make. The provider's mark says everything the cloud said and says whose as well. No state is lost, because the mapping is one to one. `.notDownloaded` is exactly the state in which the question "from where" has an answer worth showing, and the other two arms are untouched: a downloaded item still draws nothing, a downloading one still draws the spinner. `streamingResource`, not the first media-server link. It is the same value `downloadRemoteFiles` picks, so the badge can never name a server the tap won't reach — and it is already Hardcover-filtered, which matters here: the API's `markExternalSourceUploaded` marks EVERY provider row of an item 'downloaded', so a dual-linked book's Hardcover row passes a bare syncStatus check. That once aimed a download at a provider with no files at all. With the corner carrying provenance, the subtitle goes back to what develop has — glyph and separator deleted, `Text(verbatim: item.details)` inline again, and the doc comment justifying the `Text` concatenation with it. The trade accepted knowingly: provenance is now visible only while the file is remote. Once it is downloaded the book plays the local file and where it came from stops mattering to the reader. It does NOT stop mattering to the app — progress still pushes to that server, keyed off the resource's providerName — which is why `getAccessibilityLabel(includeSource:)` stays unconditional. VoiceOver never saw the badge in any state, so the announcement is that user's only channel. The geometry is measured against the cloud rather than eyeballed. All three badges share the cloud's 14pt-wide box, which is what centres them: a height-only frame lets each mark's own width decide where its centre falls, and these two differ enough (Jellyfin square, Audiobookshelf taller than wide) to sit visibly right of a cloud row. The 3pt below buys what the cloud gets for free — it is a wide, low glyph, so fitting it into a 14x16 box letterboxes it and leaves ~4.6pt of air under its ink, where a mark that fills its box lands flush on the corner's edge and reads as sitting lower. Heights are per-provider on `badgeHeight` because the symbols don't fill their boxes equally: at a shared frame height Jellyfin's ink draws ~0.4pt shorter than Audiobookshelf's. The ceiling is not the corner's diagonal, which nothing reaches until well past 13, but the ink's centre climbing away from the cloud's.
| public let jobType: SyncJobType | ||
| public let uuid: String | ||
| public let relativePath: String | ||
| public let parameters: [String: Any] |
There was a problem hiding this comment.
🔵 INFO — QueuedSyncTask carries public let parameters: [String: Any] and is returned across the SyncQueueRepository actor boundary and captured into detached Tasks in SyncQueueService. It is runtime-safe today (only value-typed dict entries are read across the hop), but the [String: Any] payload defeats Sendable checking on a type that legitimately crosses actor/Task boundaries — a latent gap if the target moves to Swift 6 strict concurrency. Consider a typed payload or an explicit @unchecked Sendable with a rationale.
| let message: String | ||
| let error: String? | ||
| } | ||
|
|
There was a problem hiding this comment.
🔵 INFO — upload(fileURL:remoteURL:) streams a potentially multi-GB audiobook over URLSession.shared (a foreground session), so the S3 PUT won't survive app termination/suspension mid-transfer. The call site (LibraryItemSyncOperation) documents this as intentional with a background-transfer follow-up planned — noting it here so the BPURLSession background-session migration isn't lost.
This was referenced Sep 23, 2026
This was referenced Sep 24, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #1523 — @Hirobreak's External Resource work, squash-rebased onto current develop (the original fork branch has maintainer edits disabled and had diverged ~70 commits from its July base). Feature authorship is preserved on the squash commit.
What this adds
Cross-device sync of media-server books: an item imported from Jellyfin/AudiobookShelf carries an
ExternalResource(provider, provider item id,hostId) that round-trips through the BookPlayer API — a second device resolves the resource to its own saved connection and streams directly from the user's server. Progress pushes back to the media server run on the new unifiedConcurrenceServicetask queue (SwiftData schema v3), and connections now capture the server's stable GUID at sign-in (all three flows: Jellyfin password, Quick Connect, ABS login). CoreData migrates v11→v12 with a mapping model. The LITE tier lands on iOS (AccessLevel.lite,hasSyncEnabled = pro || lite) with per-job access gating.The cross-platform hostId contract (Android shipped this in 1.1.3)
hostId := <server-reported GUID> ?: canonicalDedupKey(url); resolution is provider-scoped: GUID match (case-insensitive) → canonical URL key → nil, never guess a server. Unresolvable playback surfaces the Media Servers shortcut; unresolvable progress pushes are consumed/discarded (an unknown host is permanent on-device, and the queue retries failures forever).On top of the rebase, this branch fixes review findings from comparing against the Android implementation
IntegrationHostResolverreplacing per-site lookups that compared GUIDs case-sensitively against rawabsoluteStringand then fell back toconnections.first— cross-device that streams the wrong file or writes progress to the wrong server.hostIdwrites usestableHostId(serverId ?? canonicalDedupKey) at all six import sites.PlayableChapterno longer encodesexternalUrl/externalHeaders: the headers carry the media server's liveAuthorizationtoken, and encodedPlayableItems reach the WatchConnectivity application context, which is persisted to disk on both devices.externalUrl != nil, hiding it in exactly the connect-your-server case).externalUpdateis available on every tier (Android parity — the push targets the user's own server, not a billed resource).Verification
Phone + watch schemes build; 530 unit tests, 0 failures. Browser-SSO stack (
WebAuthenticating/OIDC/PKCE) is guarded iOS-only in the shared framework; its window lookup uses a dynamicsharedApplicationread because BookPlayerKit compiles extension-safe.🤖 Generated with Claude Code
https://claude.ai/code/session_01YNhXd8EkXcrjBVYLHm5ZSp