codex/bound-transcode-segments
101
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Quick104
|
fd3d737a58 | fix(playback): trust completed segment writes | ||
|
Quick104
|
dffa6ae34e | Merge origin/main into codex/bound-transcode-segments | ||
|
Quick104
|
e2899ff619 | fix(playback): preserve retained segments across restarts | ||
|
Puks The PirateandGitHub
|
20c6b146eb |
fix(jellycompat): repair episode handoff (#529)
Continue Watching clients can retain stale user and media-source values across episode transitions. Keep token-derived authorization authoritative and fall back to the requested item files. Strip NUL code points across durable playback-session JSON so PostgreSQL persistence cannot leave negotiated sessions cache-only. |
||
|
|
881c96864b |
feat(playback): finalize platform-neutral protocol v3 (#567)
* docs(playback): add v3 neutral-contract finalization plan Supersedes the wire-contract sections of the 2026-07-12 v3 plan: server-owned attempt keys, delivery-keyed negotiation without Media3 engine names, tiered capability evidence, neutral device/output context, track/quality replan operations, audio-only planning, and coordinated no-back-compat rollout across server, Android, Apple, and web. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(playback): make v3 attempt keys server-owned and replace engines with deliveries Contract core of the platform-neutral v3 finalization (plan sections 3.1 and 3.2), breaking on purpose — v3 is dark and all clients move together: - Every PlanV3 now carries plan_attempt_key, an opaque server-computed token clients store and echo in attempted_plan_keys; ReplanRequestV3 gains bounded local_mutations that the replan handler folds into the failed plan's key. Clients never hash anything. - KotlinName() is deleted from DeliveryV3, StreamProtocolV3 and SubtitleModeV3; the attempt-key canonical string now uses lowercase wire tokens, and PlanRecipeVersionV3 bumps to v3.3 so no key or plan ID computed under the old canonicalization can collide. - EngineV3 leaves the wire: ClientPlaybackContextV3.Engines (media3_*) becomes Deliveries keyed original_http|progressive|hls, with EngineCapabilityV3 renamed DeliveryCapabilityV3. PlanV3.Engine is removed; the planner, subtitle policy and quirk registry re-key on delivery class, and the media3_only feature token is deleted. - Validated-claim strings drop the prefix: media3_h264_decode -> h264_decode, media3_audio_decode -> audio_decode. - Golden fixtures in testdata/protocol_v3 are regenerated by Go and are now the cross-repo source of truth. Part of the playback protocol v3 neutral-contract train (steps 2-3 of docs/superpowers/plans/2026-07-30-playback-protocol-v3-neutral-contract.md). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(playback): add v3 evidence tiers and neutral device/output context Implement plan sections 3.3 and 3.4 of the v3 neutral-contract pass: - ClientCodecCapabilitiesV3 gains required video_evidence and audio_evidence closed enums (exact | platform_attested | declared). Planner strictness follows the tier: exact keeps the strict decode-entry validation, platform_attested validates codec/resolution/bit-depth/ frame-rate but skips profile/level matching, declared grants copy routes from the flat codec lists. Only exact audio evidence earns passthrough claims. The detailed_decode_capabilities feature token is deleted (subsumed by video_evidence=exact), and evidence-blocked direct routes carry the new evidence_insufficient_for_direct reason/warning. - DeviceContextV3 is now platform/os_version/manufacturer/model plus a bounded platform_details map (<=16 entries, <=128 chars); the Android Build dump fields are gone. Fire TV quirks keep matching on manufacturer/model (brand fallback removed with the field). - output_route_generation (int64, dual-location) becomes an optional opaque output_context_id string on the output context; the dual-location consistency validation is deleted. Attempt keys, plan invalidation, route events, and the planstore column follow (new Goose migration). - Feature advertisement collapses to the top-level client_features list only; ClientPlaybackContextV3.Features is deleted and ReplanRequestV3 gains an optional client_features refresh. - PlanRecipeVersionV3 bumped v3.3 -> v3.4; fixtures re-keyed. Part of the playback protocol v3 neutral-contract finalization plan (docs/superpowers/plans/2026-07-30-playback-protocol-v3-neutral-contract.md). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(playback): add v3 intent replans, quality menu, and audio-only routes Protocol v3 could only replan after a failure, so changing the audio track or the quality still required the legacy audio PATCH and the client-recipe transcode start — the two endpoints v3 is meant to replace. Clients also had to own a resolution ladder to render a quality menu, and a source with no video track was terminaled by the video/HDR gates, keeping audiobooks on the legacy path. Add track_change and quality_change replan operations. They carry no failure classification and route through the existing replan transaction, so they inherit its idempotency, capacity reservation, and staged-successor commit for free. Because nothing failed, the previous route stays eligible: neither the attempted-key history nor the failed-plan exclusion applies to them. Publish the server ladder on the plan as available_qualities so the quality menu is server-owned; the rungs come from the same resolutionLabelV3 and ladderBitrateKbpsV3 helpers the planner itself uses, not a parallel table. Plan audio-only sources through their own reduced route family: original_http when the client decodes the codec, otherwise a progressive AAC conversion. The plan advertises audio/mp4 for that remux and the transport now serves the same value, because a declared-tier client probes the advertised MIME with isTypeSupported before attaching a source buffer, and "video/mp4" on a stream with no video track is exactly the mismatch that makes the probe lie. Name the protocol's string vocabulary (dynamic ranges, transformations, executors, validated claims, terminal reasons) as constants while touching these lines, so the wire values have one definition. Part of #135 * docs(playback): publish the v3 protocol contract and fix subtitle ordinals Protocol v3 exists only as Go code today, so the Android and Apple ports have no authority to implement against other than reading this repository. Publish the contract as a normative document, machine-checkable schemas, and generated golden fixtures, and fix the one place where the server's own wire output disagreed with the ordinal space it publishes. - docs/architecture/playback-protocol-v3.md is self-contained enough for a third-party client: endpoints and status codes, evidence tiers and their bound-matching rules, delivery classes, the timeline model, replan semantics, registries, track identity, plan identity, quality, and transformations. - docs/design/schemas/playback-v3/ carries JSON Schemas for the five wire shapes plus valid and invalid fixtures, following the client-diagnostics layout. internal/playback/contract validates every fixture against its schema, so a schema that drifts from the Go types fails the Go suite. - cmd/playbackfixtures generates internal/playback/testdata/protocol_v3 from the production planner. `make playback-fixtures` writes them and `make verify-playback-fixtures` (wired into CI) fails when they are stale. These files are what the client ports consume, so drift would otherwise surface as a playback bug on three platforms at once. The subtitle fix: combined ordinals are one dense space over externals, then embedded tracks, then downloaded ones, but the legacy URL builder skipped burn-in-only tracks while assigning indices, so every track after a DVD/DVB track was numbered one too low and resolved to its neighbour. Ordinal assignment now lives in playback.BuildSubtitleInventoryV3 and both the plan inventory and the legacy `subtitle_urls` shape project from it; the legacy shape still filters burn-in-only entries but keeps each track's real index. Part of #135 * feat(web): migrate the players to the neutral playback v3 contract The web player was the last client still speaking the legacy start protocol: it picked its own file version from a codec probe, posted an ffmpeg recipe to start a transcode, PATCHed an endpoint to change audio tracks, and derived its own quality ladder. None of that survives a server-owned plan, and none of it produced telemetry the apps could be compared against. Video player: starts with a v3 request that advertises `declared` evidence from `isTypeSupported` probes and the three delivery classes, then consumes the returned plan for its URL, timeline, tracks and warnings. Quality and track changes become replans (`quality_change`, `track_change`), the quality menu renders `available_qualities` instead of computing rungs, and playback failures emit `route-events` so web failures land in the same diagnostics as Android and Apple. The duration comes from `source.duration_seconds` rather than the playback engine, and the "how was this delivered" overlay reads the plan's delivery and server transformations instead of comparing codec strings. Audiobook player: starts against the audio-only planner path with a single `original` rung, and takes its seek anchor from `timeline.player_start_seconds` so the progressive-remux route (which anchors the stream and restarts the player clock at zero) does not seek twice. Server side, `disable_progress_persistence` left the wire, so the rule it encoded is now derived. Resume state is keyed on the item, but every part of a multipart presentation shares that key while carrying its own file-local clock — persisting part 4's position would store "12 minutes in" as the book's resume point. `PresentationPartTotal > 1` expresses that directly and generalizes to multipart movies and split episodes, and a client can no longer forget to ask or lie about it. `useTranscodeQuality` and the legacy response types are deleted, and `WEBTEST_KNOWN_FAILURES` loses the audiobook entry along with its fix. Part of #135 * feat(playback)!: make v3 the only playback protocol Protocol v3 shipped behind a flag, alongside the legacy start path it was designed to replace. Running both meant every planner change had to be made twice, in two shapes that disagree about who decides the route: the legacy body carried a decision the client had already made, while v3 asks the server to make it. This deletes the legacy half. Removed: - `handleStartPlaybackLegacy` and its request/response bodies. The `POST /playback/start` route stays, but the protocol-version dispatch envelope is now a strict v3 decode — a body that does not declare `protocol_version: 3` gets `426 client_upgrade_required` so an outdated app can render a clear "update required" state instead of misreading a plan. Deliberately not a `400`: the request may be well-formed for the protocol it was written against. - `POST /playback/transcode/start`, superseded by the `quality_change` replan operation, and `PATCH /playback/{session_id}/audio`, superseded by `track_change`. Both mutated a session without re-planning. - The shadow planner and both rollout settings rows. With v3 the only protocol, `playback.protocol_v3_enabled` would mean "no playback at all"; `playback.protocol_v3_shadow_enabled` gated a comparison against a path that no longer exists. `409 protocol_disabled` on route-events goes with them, and capability `enabled` is now constant `true` (the field stays — clients feature-detect against it). - Version-selection helpers in `internal/playback/resolver.go` that only legacy start reached. `Resolve`/`ClientCapabilities`/`PlayDecision` stay: downloads consumes them. `internal/jellycompat` has its own resolution surface and is untouched. Behaviour the legacy handlers owned and v3 now owns explicitly: series version and audio-track preferences are persisted on start and on a `track_change` replan (not on failure recovery, whose forced route is not a user choice); an omitted `start_position` resolves to the profile's saved resume point; and an omitted audio track resolves through the series preference, the profile audio language, then the library override. Both are settled before planning, because the plan's timeline is cut at the start position. Spec §2.2 documents this as "omission is a request, not a default". The encode-target clamp that lived in the deleted transcode handler is already enforced in the planner, twice — `availableQualitiesV3` omits rungs at or above the source height, and the encode path clamps `targetHeight` to it. Unchanged: progress, stop, HLS manifest and segment delivery, the realtime control socket, stream tokens and restart reconstruction, watch together, downloads, jellycompat. Every removal is recorded in the pre-lock removals table in docs/architecture/v1-scope.md. Part of #135 * fix(scanner): stop recording embedded cover art as a video track ffprobe reports embedded cover art as a video stream carrying disposition.attached_pic. convertProbeData appended every "video" stream to VideoTracks without consulting isMainVideoStream, the predicate that already existed for duration decisions, so the picture was persisted as a playable track. That misreports the file twice: - An audio file with a cover picks up a video track, so it no longer satisfies MediaFile.IsAudioOnly and the v3 planner routes an audiobook through the video path instead of planAudioOnlyV3. - When the picture is ordered ahead of the real stream, the flat codec_video/resolution/hdr columns describe the poster: a 954x720 h264 episode was stored as mjpeg 480x480. Filter attached_pic streams out of the track loop. The guard is the disposition flag, not the codec name, so a genuine MJPEG video is still probed as video — the library has one. Already-probed rows self-heal on the next playback: NeedsCriticalProbeRepair already reprobes tracks missing color_range, which covers 21 of the 23 affected rows, and applyProbeData overwrites VideoTracks wholesale. The remaining two need a rescan; nothing persisted records attached_pic, and keying repair off still-image codec names would reprobe the genuine MJPEG file on every playback forever. Part of the playback v3 neutral-contract work: it is what lets Android drop AUDIOBOOK_COVER_ART_CODECS, which fabricated decode support the client cannot honestly claim under video_evidence: "exact". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(playback): publish subtitle URLs even when playback starts with subtitles off The v3 plan's subtitle inventory is the authoritative track list a client builds its subtitle menu from, but the handler only rewrote it with session-scoped URLs when a track was actually selected. A start or replan that resolved to `subtitle.mode: "off"` therefore returned the planner's URL-less inventory, so a client whose picker reads the inventory had a menu it could not fetch anything from. The Cast path hits this every time: it starts with subtitles off and needs the receiver's text tracks up front. attachSubtitleArtifactV3 now scopes and publishes the inventory unconditionally and gates only the artifact stamping on the selection. Spec §8 records that the `url` on a sidecar entry does not depend on the current selection. Part of the v3 neutral-contract finalization. * chore(playback): reconcile neutral v3 with main * fix(playback): preserve subtitle intent across replans * fix(playback): retain subtitle inventory on adapted routes * fix(playback): software-decode High10 AVC for QSV * fix(playback): scale High10 frames before QSV upload * fix(playback): preserve empty subtitle inventories * fix(playback): freeze terminal attempt contract * chore(playback): name fixture contract tokens * fix(playback): close v3 conformance review gaps * chore(playback): name conformance category * fix(playback): complete v3 conformance contract * fix(playback): keep schema fixtures generated * fix(playback): emit schema-valid conformance arrays * fix(playback): omit empty replan failures * fix(web): omit empty replan failures * fix(playback): close neutral v3 contract gaps * fix(playback): harden v3 replan, transcode, and quality-ladder edge cases Review remediation for the neutral v3 cutover, server side: - A failed replan no longer overwrites the durable StartResponse with a terminal or advances the replan request ID; an idempotent start replay of a still-healthy session returns the original plan. - SoftwareVideoDecode is now derived inside the transcode layer from source facts (codec/profile/bit depth) carried on TranscodeOpts, so jellycompat, downloads, recipe-card reconstruction, and transcode nodes get the High10 software-decode fix, not just the v3 handler. video_to_h264 recipe version bumps to 2 so mixed-version node pools that would silently drop the flag fail validation instead. - Local transport startup shares the 30s ManifestStartupTimeout; a timeout with the process still running stays retryable and is no longer persisted as a durable terminal against the attempt. - Sparse replan bodies (failure_recovery et al) no longer reset a user-selected quality preference to auto; the empty-value guard now covers every operation. - availableQualitiesV3 publishes no fixed rungs when the source height is unknown, keeping the no-upscaling ladder contract. - The proxy remux path serves audio-only fMP4 as audio/mp4 via a new additive AudioOnly token claim, matching the integrated path. - Plain text subtitle sidecars accept any requested extension again (served as VTT), restoring the permissive v1 behavior; ASS and bitmap handling is unchanged. - The 4K-disallowed terminal message discloses when a lower-resolution alternate exists but was pinned away by quality "original". Part of #135. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(web): keep playback alive through failed replans and honest audio claims Review remediation for the neutral v3 cutover, web player: - A failed or refused replan no longer unmounts the player: the fatal error screen is reserved for loads with no adopted plan, and replan failures surface through the existing non-fatal replanError path. - changeQuality rolls its optimistic preference back when the replan is refused or errors, so a failed switch is not silently applied by the next unrelated replan and the menu shows the real active rung. - The capability probe now tests mp3/vorbis codecs and mp3/flac/ogg containers (MediaSource with a canPlayType fallback), restoring direct play for mp3 audiobooks instead of per-part AAC re-encodes. - Reanchor seeks issued while a replan is in flight coalesce and run when it settles instead of being silently dropped with the scrubber pinned to a phantom position. - Subtitle refresh/translation replans use the resume anchor while the media element has no metadata, so a subtitle_ready broadcast during startup no longer restarts a resumed stream at 0:00. - An exhausted failure-recovery chain sets a visible error instead of returning silently. Part of #135. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(playback): accept video-only and VP9 probe metadata Treat audio and video probe completeness independently so legitimate video-only assets converge without repeated ffprobe repair. Allow unknown codec profile/level metadata to fall through to server adaptation while preserving exact direct-decode constraints. Fixes #574 * fix(playback): address protocol v3 review findings * fix(playback): harden lease and probe repair decisions * fix(playback): close remaining v3 review gaps * fix(playback): recover failed transcode starts * fix(playback): address remaining review-bot findings on v3 replan and audio planning Server: - The deferred replan lease release is bounded by a 3s timeout so a saturated pool or DB outage cannot wedge a handler goroutine that holds the per-session store lock on an uncancellable context. - planAudioOnlyV3 honors the request bandwidth cap: an over-cap source skips the original_http direct route and converts to AAC with the same bandwidth_cap_applied warning and decision reason the video ladder uses. Unknown source bitrate never triggers the cap. - A copy-audio progressive plan rejected only by a per-delivery audio_decode_codecs subset retries as an AAC conversion instead of returning adaptation_unavailable, and the AAC recipe respects the delivery's max_channels. Web: - failure_recovery replans issued while another replan is in flight queue (superseding a pending seek reanchor) instead of being silently dropped with the fatal overlay already suppressed. - A terminal response to a fresh non-preserving start clears the previous plan and stops its session, so episode navigation cannot keep rendering the prior item under the new title. - A refused recovery replan for a transport-dead plan surfaces the error and re-arms the plan failure key, so transient recovery failures no longer strand an endless spinner; the audiobook player gets the same guard reset. - A track-less subtitle_translation_completed hands off to the refreshed persisted track once the inventory settles, clearing the live overlay, instead of pinning the synthetic live track forever. Part of #135. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(playback): reuse HLS transport for sidecar replans * fix(playback): stabilize copy HLS remount timeline * fix(playback): address v3 review findings * fix(playback): satisfy player contract types --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Quick104
|
f524c48d19 | fix(playback): close retention gaps across ranges and seeks | ||
|
Quick104
|
c26ab22809 | test(playback): strengthen retention regression coverage | ||
|
Quick104
|
0a3adf6552 | fix(playback): harden segment retention lifecycle | ||
|
Quick104
|
6a3fcef165 | fix(playback): prune downloaded transcode segments | ||
|
QuickandGitHub
|
31a26b2554 |
fix(scanner): accept corroborated long video durations (#545)
* fix(scanner): accept corroborated long video durations * fix(scanner): harden long duration fallbacks * fix(scanner): address long duration review feedback * fix(scanner): align long duration seek semantics * fix(scanner): align legacy long media seeking * fix(scanner): preserve long timeline origins * fix(scanner): preserve long audio-switch timelines * fix(scanner): distinguish absolute-end timelines * fix(scanner): preserve long resume semantics * fix(scanner): reject ambiguous absolute ends * fix(scanner): preserve compat real-manifest resumes |
||
|
QuickandGitHub
|
73488d1bfa |
feat(metadata): add original-language preferences (#526)
* feat(metadata): add original-language preferences * fix(settings): show metadata exceptions immediately * docs(settings): add metadata language screenshot * fix(settings): make language exceptions responsive * fix(settings): standardize language display names |
||
|
|
dc4b9a0909 |
feat(settings): add the cross-platform settings contract and its manifest (#479)
* docs(settings): define the cross-platform settings contract Turns the audit in #376 into a decision-complete design for how user settings work across the server, bundled web client, Apple clients, and Android clients. Today there are three partial contracts - the server registry, the web client's own manifest, and independently owned key constants in each native client - and they have measurably drifted. The root enabler is that keyUsesUserScope returns true for any unregistered key, so a client can invent a production setting unilaterally and the server stores it as an unvalidated string. The design decides: Ownership. Every production user-facing setting needs a server-owned manifest entry, even when the value is stored only on one client. The single exception is private local.<client>.* diagnostics, bounded by five conditions. Types and scopes. Native JSON values instead of strings. Five remote scopes plus client_local, and each definition declares its own resolution order rather than inheriting a global precedence. Preferences versus restrictions. internal/policy already resolves max_playback_quality and metadata-language limits over the same controls this contract resolves preferences for. Definitions declare constrained_by, the effective response reports the permitted value alongside the user's stored one, and a mutation exceeding a restriction is stored rather than rejected - a capped 4K preference should take effect the day the cap lifts, not be destroyed by it. Compatibility. Widening a scope, adding an enum member, or widening a range is additive and revision-tagged; narrowing anything needs a new key. introduced_in is a manifest revision attached to individual enum members and scopes, not just whole definitions, so a newer client never offers a choice an older server will reject. Rollout. One coordinated breaking release, with no compatibility shim, projection, or client fallback. After the cutover no future setting requires coordination. No settings version check goes in the authenticated middleware and nothing returns 426: deleting the old routes already produces the break, and a gate would be more code in four repos for the same outcome while permanently coupling every endpoint to one subsystem's versioning. Scope placement. Appearance and date/time move from account to profile scope. Account scope was an artifact of pre-profile storage; leaving it there means a household shares one theme and text size, and any non-child profile can restyle everyone else. Read path. Batched context resolution, index requirements, a session-snapshot rule, and a no-regression benchmark gating storage consolidation - profile_series resolution is per-item, so a season view would otherwise issue one request per episode. Verified against the current server, Apple, and Android implementations. Two findings shape it: the unknown-key extension bag is real, and v1 scope reads NOT LOCKED, so removing the legacy surface needs no amendment if it lands before lock. Related to #376. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(settings): add the canonical settings contract manifest First implementation step for the cross-platform settings contract (#376). Adds the artifact everything else depends on: the manifest, its JSON Schema, the object value schemas, and a Go loader that validates the whole thing at load time. No routes, no storage, no behavior change — nothing reads this yet. contracts/settings/v1/ holds the artifact at a stable path because clients vendor it and generate bindings from it. The embed directive has to sit beside it (go:embed cannot reach outside its own directory), so that directory is a tiny Go package containing nothing else; loading and validation live in internal/settingscontract. 38 definitions: 35 remote, 3 contract-known client_local. That covers every key the legacy registry accepts, every unregistered key the extension bag was silently accepting from the web client, every unregistered device key Android writes, and the profile preference columns that become settings. Registering the previously-unregistered keys is where the drift shows up, and the manifest records each case in a notes field: - ui_theme, ui_text_scale, ui_text_weight, ui_high_contrast, ui_custom_theme_vars, and ui_custom_css reached the server only because keyUsesUserScope returns true for any unregistered key. They are now typed, renamed to the dotted convention every other key uses, and moved to profile scope per the design. - player.match_frame_rate and player.sleep_timer_default_minutes are written by Android against a server that does not register them, so every write and reset is currently rejected. Registered. - player.next_up_prompt_seconds is Android's alias for playback.next_up_prompt_seconds and does not become a definition; the test matrix pins it as a migration alias. - player.playback_speed is capped at 3.0, matching the server rather than Android's 4.0. - subtitle_appearance becomes playback.subtitle_appearance. Every other canonical key carries a domain prefix, and preserving accidental key names is an explicit non-goal of the design. Validation is deliberately stricter than the schema can express. Beyond shape, it enforces that a resolution order ends in "default", that it only resolves scopes the definition allows, and — the one most likely to bite — that every writable scope is actually read, so a setting cannot accept writes at a scope it will never honor. Defaults are validated against their own value schema, so a default that violates its own range or enum fails at load. Revision tags are checked to never run ahead of the manifest revision, which is what makes revision-aware client filtering trustworthy. Ceiling and floor policy constraints are rejected on unordered types, where capping would silently do nothing; playback.preferred_quality's enum is therefore ordered ascending. ValidateValue is the single validation path, so the mutation endpoint, the migration, and the manifest's own default checks cannot diverge later. Numbers decode through json.Number so an integer setting rejects 30.5 rather than truncating, and object values validate against their referenced JSON Schema instead of accepting arbitrary JSON the way validateJSONSetting does today. Canonicalization implements RFC 8785 over the value domain the contract uses: sorted keys, no insignificant whitespace, ECMAScript number formatting. The digest is the ETag, and PublicBytes strips maintainer notes so the served manifest never carries internal commentary. Promotes santhosh-tekuri/jsonschema/v6 from indirect to direct. Verification: 124 tests pass across 16 cases; golangci-lint clean; make verify-local-paths passes. Two failures in internal/api/handlers (TestRemoveJellyfinCompatWebDisablesWebSetting, the playback v3 seek recovery test) reproduce unchanged on main and are unrelated. Part of #376. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(settings): give ui.theme a device override Theme joins text scale, text weight, and high contrast as a profile default with an optional per-device override, resolving profile_device -> profile -> default. The right theme is partly a function of the screen and the room — a light theme on a phone in daylight, a dark one on a TV at night — which is the same reasoning the other three appearance keys already used. All four appearance settings now cascade consistently, which also means one rule to explain in the UI rather than "these three follow the device, that one does not". ui.custom_theme_vars and ui.custom_css stay profile-wide. They are authored styling rather than a contextual preference, so a profile's custom tokens still apply on top of whichever theme a device resolves to. Recorded in the definition notes because it is a visible consequence: vars tuned against a dark theme will sit on top of a light one if a device overrides the theme. Widening those to profile_device later is an additive revision bump if it turns out to matter. Part of #376. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(web): tag local appearance caches with their owning account The theme, text scale, text weight, high contrast, custom theme variable and custom CSS caches in localStorage were untagged, so on a shared browser a second account inherited the first account's appearance: with no server value of its own, every fallback resolved to whatever the previous account had stored, and the leftover `silo-theme` key also suppressed the admin-configured default theme for the new account. DateTimeFormatProvider already solved this by stamping its cache with the authenticated user id and refusing another account's values. Extract that mechanism into `createOwnedCache` in utils/storage.ts (where key namespacing lives) and put all three groups behind it, so appearance and custom theme get the same protection instead of a third copy of the rule. - Each group carries its own owner stamp. A shared stamp would be unsafe: the groups are written by hooks nested inside each other, and effects run inner-first, so whichever hook stamped first would vouch for the other's still-stale values. - A null owner (auth bootstrapping, or signed out) still trusts the cache, which keeps the warm start and the login screen's last look. - An unstamped cache is not trusted once an account is known, so existing users take a one-time appearance reset on first load rather than a chance of seeing someone else's settings. - When a foreign cache is detected the values are dropped and the empty cache is handed to the new account, so a later single save cannot re-trust the rest of the previous account's state. Owner is the user id because /settings is user-scoped server side; it lives in one helper (`appearanceCacheOwner`) so it can be widened if appearance moves to profile scope. `shouldLoadApiTheme` is gone: it had become a synonym for `appearanceCacheOwner(...) !== null` with no callers left. Part of #376 AI-use disclosure: implemented with Claude Code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(settings): make the settings contract enforceable and fix the appearance cache The contract manifest landed as a document nothing checked. This makes it a mechanism, and fixes the one defect in the change set that hurt users on merge rather than at cutover. Web appearance cache. useTheme cleared the cache for any account whose stamp did not match and never repopulated it — the only writers were the four user-action setters — so every upgrading user lost their warm start on every load, not once, and x-large-text and high-contrast users lost theirs too. The owner-stamp protocol is replaced with per-account key namespacing (`silo-theme:7`): a foreign value is absent rather than present-and-distrusted, so nothing has to be deleted, the first account keeps its warm start, and there is no shared stamp for a second tab, a stale debounce timer, or an out-of-order effect to race on. Widening ownership to profile scope, which this manifest requires, is now a change to appearanceCacheOwner alone. Adds the API-to-cache mirror useTheme was missing, cancels pending debounced writes across an account change, and re-seeds provider state during render so no frame paints the previous account's look. Canonicalization. writeCanonical used json.Marshal, which HTML-escapes < > and &, and canonicalNumber used Go's 'g' format — both diverge from RFC 8785, so the first label containing an ampersand or bound below 1e-4 would have forked the server's ETag from every conforming client. Output is now byte-identical to ECMAScript String() across the edge cases, verified against node. The ETag also covers the value schemas, which decide what the server accepts and previously could change while the tag stood still. All four derived representations are memoized; a conditional GET no longer costs a full parse and re-serialize. Validation. strictUnmarshal's decoder.More() answered false for a stray ] or }, so `true]` validated as a boolean. Enum matching compared fmt.Sprintf tokens, so the string "3" satisfied an integer member. Declared steps were never enforced. The language pattern rejected tags both mobile platforms emit unprompted (en_US, ca-ES-valencia, ar-EG-u-nu-latn) and never normalized case, so en-US and en-us were two rows for one preference; NormalizeValue now canonicalizes on the shared path. Manifest. show_forced_subtitles defaulted false where the server column is NOT NULL DEFAULT true, which would have turned forced subtitles off for every profile that never touched it. preferred_quality declared 13 members where the planner speaks 6 and collapses the rest to auto. metadata_language's allowlist was bound to the very column it migrates from. subtitle-appearance pinned fontFamily to three families while Apple stores any installed system font. Registers five user-facing settings the clients already ship, and corrects three notes that described Android behaviour that was not true. Enforcement. The package had no non-test callers, so MustLoad never ran; it now loads and logs at startup. The inventory test compared the manifest against a hand-copied map and could not see the drift it named; it now iterates settingsRegistry and checks defaults too — both verified to fail on injected drift. Adds .github/workflows/ci.yml, the repo's first CI that runs go test, go vet, gofmt, and the frontend suite. Known pre-existing failures are named individually in the Makefile so everything else stays gated and the list can only shrink. Part of #135 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(settings): align the sleep timer default and range with the shipped client Android is the only client that implements this setting. It clamps to 0..240 and defaults to 30. The manifest said 0..480 with a default of 0, so a manifest-driven UI would have offered durations no client can store, and every user who never opened the picker would have had the preset silently turned off at cutover. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * ci: give the new workflow the deps it actually needs The first run exposed two gaps in the workflow itself. go build ./... fails without libvips headers, because h2non/bimg binds libvips through cgo and pkg-config; the Dockerfile installs the same package. And pnpm/action-setup resolves its version from package.json, but there is no package.json at the repo root — the packageManager field lives in web/package.json, and a job's defaults.run.working-directory does not apply to an action's inputs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(web): stop the diagnostics download test depending on the Node version new Response(blob) reads the body through blob.stream(), which jsdom's Blob does not implement on Node 22 — the version the Dockerfile builds with. The test passed locally on Node 24 and threw "object.stream is not a function" in CI. Nothing in it asserts on the body, only that the object URL and filename reach the anchor, so a string body is equivalent and works on both. Surfaced by the CI workflow added in this branch, which is the first thing in this repo to run the frontend suite anywhere but a developer's machine. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(build): copy the settings contract into the container build context Both Dockerfiles copy cmd/, internal/, migrations/ and web/embed.go, but the manifest lives in contracts/settings/v1 — an embedded Go package that sits outside internal/ because clients vendor those files. The image build therefore fails with "no required module provides package .../contracts/settings/v1". Caught deploying to the dev box. Nothing had built an image since the manifest landed: the Docker workflow only runs on pushes to main and workflow_dispatch, and CI's go build runs against a full checkout, so neither gate covers the container context. This would have broken the published image on merge. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(settings): enforce the language-tag and step constraints the manifest declares A sweep of all 43 manifest definitions against the running server (160 checks: declared default, both boundaries, and deliberate violations for each remote key) found two places where the live registry accepts what the contract forbids. Both are fixed by calling the contract's own validators rather than adding a second implementation. playback.audio_language was checked as "32 characters or fewer", so the server stored "!!!" for a field the manifest declares as language_tag — a value track matching would then silently never match. It now requires a well-formed tag via settingscontract.NormalizeLanguageTag. The empty string is still accepted: the string-only endpoint has no way to send null, and both Android and web send "" to clear the choice, so rejecting it would break clearing the preference. player.playback_speed declared step 0.05 and nothing enforced it, so 0.26 was stored — a value no client's stepper can represent and that every client would silently snap on the next write. settingscontract.StepAligned is now exported and used by both the contract validator and the registry, so there is one definition of "on step" rather than two that can drift. This gives the contract its first production consumer beyond the startup load, which is the direction Phase 2 continues in. Also fixes a genuinely flaky test that the new CI gate would have hit intermittently: TestRemoveJellyfinCompatWebDisablesWebSetting used t.TempDir as the install root, but the endpoint returns 202 and its goroutine keeps writing there after the test body returns, so cleanup tripped "directory not empty" roughly one run in four. Confirmed pre-existing and unrelated to settings; the suite now passes six consecutive full-package runs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(settings): keep widened numeric bounds resolvable at older revisions A bound was one scalar plus the revision that introduced it, which discards the value it replaced. Widening a maximum from 240 to 480 at revision 3 left a revision-3 client with no correct answer against a revision-1 server: honoring 480 offers values that server rejects, and filtering the tagged bound out leaves the setting unbounded. Since clients are specified to filter their pinned contract against the server's advertised revision, the bound has to carry what it used to be. Bounds now hold their full history, oldest first, and AtRevision hands back the limit a given peer actually enforces. A bound nobody has widened still serializes as a bare number, so the manifest reads the same and untouched entries do not churn the ETag. Validation gains the rules the representation makes checkable: a maximum may only grow and a minimum may only shrink, history is strictly ordered, later entries must say when they arrived, and the first entry cannot predate the definition. That last rule is the lower bound allowed_scopes already enforced; the same gap is closed for enum members, which could previously claim to predate the definition containing them. Reported by Codex review on #479. * fix(settings): accept the partial subtitle appearance objects already stored The schema required all nine properties, but the current API accepts and round-trips sparse objects — settings_device_test.go stores {"fontSize":"xxlarge"} and reads it back — and the web client has always merged whatever it gets over DEFAULT_SUBTITLE_APPEARANCE. Requiring the full object would have made the cutover migration quarantine preferences users really set, or block on them. Every property is now optional and a stored value is documented as a sparse override merged over the definition's complete default. An empty object is still rejected: an override that overrides nothing is the same state as no override, which the contract represents as unset. Cross-scope resolution is deliberately unchanged. A device override still replaces the profile's object rather than merging into it, because a device override means "draw subtitles this way on this screen", not "amend the profile" — and that is what the server does today. Reported by Codex review on #479. * fix(jellycompat): scan the parent directory when a sidecar changes Autoscan matched scantrigger rejections by comparing RequestError.Message against literal strings. One of those messages became "Unsupported media file extension for library type" and the copy in handlers_autoscan.go did not, so the comparison silently stopped matching. The effect is user-visible: a Jellyfin client posting a change for Movie.nfo or poster.jpg gets a 400 and the batch is abandoned, when the sidecar should have resolved to a scan of the directory containing it. Three tests covered exactly this and had been excluded rather than read. RequestError now carries a Reason the caller can switch on. Message stays prose for the client reading the response — it is meant to be reworded, and nothing should break when it is. Also makes two tests honest about asynchronous work. The Jellyfin Web teardown deleted its install root while the operation goroutine was still writing to it, where a late write recreates a path RemoveAll already walked past; it now waits for the operation's terminal state, which required exporting CurrentWebOperation. And the direct-play If-Range test pinned size and mtime so ctime was the only remaining validator, then read it back inside a single coarse-clock tick — it failed about 85% of the time on main for a reason unrelated to what it tests, and now rewrites until the stamp moves. With those fixed, GOTEST_KNOWN_FAILURES is empty and gone: make test-go runs the whole Go suite. The one test that cannot pass yet — TestHandleReplanPlaybackV3SeekFailureRecoveryNeverChangesMediaVersion, which has failed since the commit that introduced it and describes unimplemented v3 planner behavior — carries a t.Skip explaining that where the test is, rather than a regex in the Makefile. Reported by CodeRabbit review on #479. * fix(settings): reject JSON the decoder would otherwise rewrite Two cases where encoding/json accepts input by quietly changing it, which is the one thing a contract promising byte-identical agreement between peers cannot tolerate. Duplicate object properties. jsonschema.UnmarshalJSON keeps the last occurrence, so {"fontSize":"small","fontSize":"large"} validated and stored "large". Which one wins is a property of the parser, not of the contract: a client generated against a different JSON library can disagree about what it just sent, and the canonical form cannot represent the duplicate at all. Lone surrogates. An unpaired \ud800 became U+FFFD and canonicalization reported success, so the server would issue canonical bytes and an ETag for an artifact a conforming implementation must refuse — RFC 8785 requires terminating here. Substitution also means the value read back is not the value written. Both checks run before the decode that would hide them, on the shared decodeJSON path that the manifest, its public projection and every value schema go through, and again on the object branch of ValidateValue, which uses a different decoder. Reported by Codex review on #479. * ci: gate Go lint on the lines a branch changes AGENTS.md told contributors CI ran the same checks as `make lint`, and the Go job ran only gofmt and vet. A change failing the documented Go lint gate passed all three jobs. Running the linter as-is is not an option: the tree has ~296 findings today, which is why this half of `make lint` was never enforced. Blocking every PR on a cleanup nobody has scheduled gets the gate deleted again, so CI runs with --new-from-merge-base and only the lines a branch touches have to be clean. The count can then only fall. golangci-lint is built from source at a pinned version rather than downloaded. A released binary refuses to run against a Go newer than the one it was built with, and go.mod here tracks Go closely enough that the current release already fails that way on 1.26.4. .golangci.yml declared version 2 while still using v1's issues.exclude-rules key. Current golangci-lint ignores it, so the "allow repeated strings and unchecked cleanup errors in tests" exclusions silently did not apply — 16 findings in test files that the config says to skip. Moved to linters.exclusions, which `golangci-lint config verify` accepts. The four lines this surfaced in scantrigger are fixed rather than excluded: its repeated status codes and messages are now named constants, so one condition cannot end up worded two ways. Also drops the workflow token to contents:read and stops persisting credentials in the three checkouts, neither of which any job needs. Reported by CodeRabbit and Codex review on #479. * docs(v1): record the settings removal as a pre-lock exception The design removes the legacy /api/v1/settings routes and the profile DTO preference fields, while AGENTS.md states /api/v1 is additive-only and removals go through Deprecation/Sunset. Read together those contradict. They do not actually conflict: v1-scope.md scopes the additive-only rule to "when the scope locks", and the scope is still open, so a removal taken now is in scope and there is no amendment process to invoke yet. But that reasoning lived only in the settings design, where nobody checking the API policy would find it. v1-scope.md now carries a pre-lock removals table naming what goes and why waiting is worse, and states the deadline the argument depends on: a removal listed there must ship before lock or fall back to Deprecation/Sunset. AGENTS.md points at the table and says to treat an unlisted removal as a mistake. Reported by CodeRabbit review on #479. * fix(settings): clear the remaining review findings Small, unrelated except that each was raised on #479. compileObjectSchemas parsed every non-directory file under schemas/ as a JSON Schema, so a stray editor backup or .DS_Store would panic the server at startup through MustLoad. schema_ref can only name a .json file; anything else is skipped. cmd/silo used MustLoad while the ETag check beside it and every other startup failure use log.Fatalf. It now fails the same way, so a bad contract prints an error instead of a stack trace. TestRegistryDefaultsMatchTheContract called scalarDefault before handling null, and scalarDefault rejects null as non-scalar — so the subtest skipped and the comparison after it was unreachable. A nullable contract default could disagree with a non-empty registry default and nothing failed. Confirmed by injecting that drift, which now reports it. The three appearance providers each adapted the auth context to AppearanceAuth with identical code, putting the shape of auth back in three places that widening cache ownership would have to find. useAppearanceCacheOwner now does it once. useTheme.test.ts cleared storage.KEYS between cases, but appearanceCache writes namespaced keys and an owner pointer that are not in that list, so both survived and the suite was order-dependent. It clears the store, as storage.test.ts already did. The abs_smart_collection_store comment is reworded rather than given back its SQL quotes: gofmt folds a pair of apostrophes in a doc comment into a typographic quote, which is how it became one in the first place. Reported by CodeRabbit review on #479. * feat(settings): add canonical typed storage for the settings contract The cross-platform settings contract needs one typed store behind it before a resolver, routes or a migration can exist. This adds that storage to both user-store backends and holds them to identical behavior. PostgreSQL gets user_setting_values with the scope CHECK constraints, the five partial unique indexes that enforce one explicit value per identity, and the covering indexes the one-query read path needs, plus user_setting_mutations for mutation_id idempotency and the inert user_setting_migration_rejects audit table. The per-user SQLite store gets the same shape minus user_id, since that database is already user-scoped. The UserStore interface grows the typed operations: read one explicit value at one scope, collect every candidate row for a resolution request in a single query, upsert with a revision increment, unset, and the idempotency receipt operations. The resolution read deliberately returns unranked candidates so the resolver can rank in Go — one query per request, never one per scope, which the pgx query-count test pins. Delete behavior is application-enforced. Neither backend can inherit it from constraints: the SQLite store declares no foreign keys, and library, series and device columns are not FK targets in Postgres either. Profile deletion cascades to profile-anchored values while account scope survives, forgetting a device clears its profile_device values alongside the legacy overrides, and the library/series purges remove only what is scoped to that entity. The shared conformance suite covers all of it, including the set-versus-unset distinction for false, 0, "" and null, so a divergence between the two backends fails a test rather than reaching a client. Part of #376 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(settings): pin the settings-value schema constraints in both backends Completes the storage track. The conformance suite exercises the store API, which validates identities in Go before any SQL runs — so nothing noticed whether the CHECK constraints and partial unique indexes actually existed. The one-time migration writes these rows in bulk without going through the per-request path, so the schema is the only thing guarding it. Adds constraint tests to both backends covering every scope's column requirements, rejection of an unknown scope, a profile that does not exist, non-JSON values, and each of the five partial unique indexes. Also clears the lint the storage commit did not get to: sql.ErrNoRows and pgx.ErrNoRows compared with == rather than errors.Is (which fails on a wrapped error), an unchecked rows.Close, and repeated fixture literals in the shared suite now named so a backend that confuses two scope columns fails on the assertion rather than on a typo. * fix(settings): close the review findings in the validator and the theme cache Four defects the existing tests did not reach. The web theme resolver compared the server's value against the appearance cache and fell back when they agreed, but the mirroring effect writes the server's value into that same cache — so the comparison held on the first render and stopped holding on the second, reverting an explicitly chosen theme to the default. The server's value is this account's own stored choice, so it now simply wins. The regression test re-renders rather than asserting on the first paint, which is why the original one passed. golangci-lint's exclusions.paths is a path regex, not a directory list, so a bare `web` also excluded internal/jellycompat/web_component.go, internal/webhooksync/, internal/notifications/webhook*.go and eleven other non-test files that were being linted before. Anchored. json.Number is a string kind, so `"1.5"` unmarshalled into it happily and Float64 parsed the quoted digits: a numeric setting validated as a JSON string and NormalizeValue stored the quoted form into jsonb. Rejected. The lone-surrogate check ran only on the object branch, so a lone surrogate in ui.custom_css decoded to U+FFFD on SQLite and was refused outright by Postgres jsonb — the two backends disagreeing about whether the same value could be stored. Hoisted to cover every type. The strict language-tag validation this branch added is correct, but it rejects what the shipped Android client sends; the companion fix is silo-android 4aeb78b4. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(auth): stop TestJWT_TamperedToken passing a valid signature The test overwrote the last character of the signature with "X". An HMAC-SHA256 signature is 32 bytes, so its base64url encoding is 43 characters and the final one carries only four significant bits — U, V, W and X all decode to the same trailing byte. Roughly one token in sixteen was therefore left byte-identical and validly signed, and the test failed because ValidateToken correctly accepted it. Measured at 3098/50000 (6.2%) over distinct signatures; it just failed the Go job on this branch for reasons unrelated to the branch. Flipping a character in the middle of the signature is 0/50000. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(settings): reject raw invalid UTF-8, not just escaped surrogates The previous commit hoisted the lone-surrogate check to cover every value type, but that only closes the escaped path. A raw 0xff byte inside a quoted string — what an HTTP body carries when a client encodes text in the wrong charset — is not an escape, so the surrogate scan never sees it, while encoding/json still substitutes U+FFFD and reports success. NormalizeValue then stores the original bytes, which SQLite's json_valid accepts and Postgres jsonb refuses: the same backend divergence, reached the other way. Found by the Codex review bot on the previous commit's own diff. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(settings): size the library page state bound to what the web client writes ui.library_page_state's `search` was bounded at 256 characters. The web client serializes an advanced library view as URLSearchParams, encoding each filter rule as three groups[i][rules][j][field|op|value] keys — measured at 216 characters for one rule, 518 for three, 820 for five. The current endpoint validates this key by checking only that it parses, so those oversized values are already stored in production. Typing them at the declared bound would have failed the migration for anyone who had saved a view with more than one filter rule, and rejected the equivalent write afterwards. Raised to 4096, which clears ten rules with room to spare while staying a real bound. The test pins it against the key shapes libraryPageSearchParams.ts actually emits rather than a round number. Reported by the Codex review bot; the lengths above were measured by calling serializeLibraryPageSearchParams, not estimated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(settings): split quality into two axes and register the orphan keys Two manifest changes the cutover needs. **Quality becomes resolution + bitrate.** The legacy ladder values (1080p-high, 720p-medium, 1080p-8, 420p, 328p) were never a third dimension — they are a bitrate spelled into the resolution string. The web player already decomposes them: useTranscodeQuality.ts defines 1080p-high as {resolution: 1080p, bitrate: 10000} and sends the two separately, so the compound form never reached the wire. Downloads went further and kept only a bitrate ladder. So playback.preferred_quality keeps the six clean resolutions and playback.max_bitrate_kbps becomes the second axis, nullable because "uncapped" is a real answer and a numeric sentinel would need widening every time hardware improves. Clients compose their own presets from the pair, which means retuning what "High" means is a client release rather than a contract break. Migration decomposes each legacy value losslessly, so none of them lands in the rejects table. **The five extension-bag keys are now definitions.** card_overlays, next_up_mode, sidebar_pins, disabled_library_ids and library_order reached the server only through the unknown-key path, stored as unvalidated strings. Two of them the server reads back — next_up_mode decides home section assembly and card_overlays falls back to an admin default — so they cannot be demoted to client-local. Registering them is what lets the extension bag close. Adds three schemas for their shapes and a test that exercises every schema_ref against a real value: each of these is nullable with a null default, so the existing default-validation test returns at the null branch without ever compiling the reference. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(settings): add the canonical resolution engine One answer to "what is this setting, for this profile, on this device, for this content". Before this, each caller carried its own ladder: catalog/detail.go resolved subtitles across four levels by hand and audio across three, handlers/settings.go had a two-level device/user resolution with a lazy write-back inside a GET, and jellycompat read profile columns directly. Those disagreed about precedence, which is the drift the contract exists to remove. Resolution is one batched read regardless of how many keys, libraries, or series are in play — ranking happens in Go against each definition's declared resolution_order. Five sequential index lookups per key per item is the implementation the design rejects, and a season view is exactly where it would have shown up. An absent identity drops its scope rather than erroring, so one code path serves an identified client, an anonymous jellycompat seed, and a batch spanning many series. Rows for a foreign profile, device, library or series are ignored even though the batched read returns them. Constraints narrow without destroying: a capped 4K preference resolves to the cap, reports itself constrained, and keeps the authored value so it takes effect the day the cap lifts. Two cases needed care — null on a nullable numeric means unbounded, so a ceiling must cap it rather than rank it equal and let the value that most needs capping slip past; and an allowlist falls back to a permitted member rather than the definition's default, which may itself be outside the list. Adds ValueSchema.CompareValues to the contract package, since ordering values is what makes a ceiling or floor mean anything and value semantics belong with the schema that declares them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(settings): add the one-time migration planner The conversion rules from legacy settings storage to canonical values, as ordinary Go rather than twice in two SQL dialects. Both backends read their own rows, hand them to Plan, and write what comes back — so the decisions are testable without a database and SQLite and Postgres cannot drift apart in what they decide. The rules that needed care, each pinned by a test: Column defaults are not choices. quality_preference is NOT NULL DEFAULT '1080p' while the contract defaults to auto, so migrating the column unconditionally would pin every profile in the install to 1080p having never chosen it — and that stored value would then outrank the contract default forever. Same for language 'en', subtitle_mode 'auto', and show_forced_subtitles true. The empty string is unset, not a value. The legacy string API had no way to send null, so both Android and web spell "clear my choice" as "". Storing that would make a cleared setting outrank the default. Legacy quality decomposes rather than rejects. Every compound value maps to a resolution and a bitrate from the ladder in useTranscodeQuality.ts, so nothing lands in the rejects table. Account rows fan out to every profile, which is the account-to-profile move the contract makes for appearance and search scope: a household that shared one theme each end up owning theirs. Legacy strings become typed JSON — "true" to true, "30" to 30 — or every generated binding would fail to decode what the migration wrote. Nullability differs per backend, so profile columns arrive as pointers and the caller resolves "chose the default" versus "never written" when it reads. jellycompat's DisplayPreferences blobs ride the same table under synthetic keys and are left alone; they are that subsystem's storage. Everything that cannot convert is recorded with a reason rather than dropped, and a final test asserts every planned row would be accepted by the mutation endpoint's own validation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(settings): run the one-time migration on the SQLite backend Wires the planner to real storage as userdb migration V15. V14 created the tables; this fills them. It runs inside runMigrations' existing transaction, so a database either comes out fully migrated or untouched — a partial migration is the one state neither the operator's backup nor a rollback covers. Pinned by a test that rolls back and asserts nothing was left behind. Two things the wiring had to get right that the planner could not see: Reject identities are JSON. Postgres declares that column jsonb NOT NULL and SQLite guards it with a json_valid CHECK, so the free-form "profile=p1 device=d1" the planner emitted would have failed to insert — on exactly the rows the table exists to record. They are structured documents now, which is also queryable. Subtitle and audio preferences are two tables keyed the same way, so they merge into one per-series record before planning. Converting them independently would have produced two rows racing for the same identity. Every legacy read tolerates a missing table, since this runs against databases created at any schema version, and preferred_metadata_language is deliberately absent: that column exists only in the Postgres schema. Tested end to end against a real database rather than only through the planner — the rows land, satisfy the scope CHECK and the partial unique indexes, and hold valid JSON. Also covers the empty-install case and asserts a second run fails rather than silently doubling every value. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(settings): run the one-time migration on the Postgres backend The mirror of userdb V15, registered with goose as a Go migration rather than SQL: the conversion validates every value against its own definition and re-encodes it as typed JSON, and one legacy quality string becomes two rows — neither is expressible in SQL without duplicating the manifest. The rules stay in internal/settingsmigrate, so the two backends cannot disagree. RunTx, so the whole backfill lands in goose's transaction. The down migration empties the canonical tables; the legacy ones are never touched by the up, which is what keeps the cutover reversible until the follow-up migration drops the superseded columns. preferred_metadata_language is read here and only here — the column exists in this schema and not in SQLite's, so this is the sole source for catalog.metadata_language. Verified against a real Postgres: the full goose chain runs, 1080p-high decomposes to ("1080p", 10000), values land as typed jsonb rather than strings (jsonb_typeof reports number), rejects carry a queryable jsonb identity, and the composite profile foreign key refuses a row naming a profile that does not exist. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(settings): add the canonical settings API The routes that make the typed storage reachable. Until now the manifest, the resolver and the migration all existed with nothing able to call them. GET /settings/contract serves the public manifest behind an ETag — clients vendor a pinned copy and generate bindings from it, so the common request asks "still the same contract?" rather than transferring it. Its capabilities sibling reports revision and supported scopes for feature detection instead of version sniffing. /settings/values/{key} reads, writes and clears an explicit value at one named scope, which is what a reset affordance needs: "did I set this here" is a different question from "what applies", and the old endpoint could only answer a blurred version of both. Scope comes from the query while profile and device come from session headers, so one profile cannot address another's settings by naming it. /settings/values/effective resolves any number of keys in one request, with the resolution ladder and the source of each answer reported so a client can offer "reset this device's override" against the exact row holding it. Asking for no keys returns every remote setting, which is what a settings screen wants. Writes are idempotent when a client sends X-Silo-Mutation-Id: a retry after a dropped response replays the receipt, and reusing an id with different content is a conflict rather than a silent overwrite of the wrong thing. Three things the string-only endpoint could not do, each pinned by a test: an unknown key is refused rather than stored in the extension bag, values are checked against their declared type and range, and a write to a scope the definition does not allow is rejected. Registered before the catch-all /{key} routes, which would otherwise swallow "contract" and "values" as setting names. The legacy endpoints stay live for now; deleting them is the next commit, once their consumers move. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(settings): generate typed bindings for all four languages One generator rather than one per repo. The point of the contract is that four codebases agree on keys, types, scopes and defaults, and four independently written generators would be four chances to disagree. Go and TypeScript land in this repo; Kotlin and Swift are written into the sibling client checkouts, skipped with a note when they are not present so a server-only developer can still run it. Output is sorted by key so an unrelated manifest edit does not produce spurious diffs. The Kotlin output is the interesting one: it generates the DeviceSettings allowlist Android maintained by hand, plus the BOOLEAN_KEYS/INT_KEYS/ DOUBLE_KEYS classification it kept as a *second* hand-maintained table that had to agree with the first. Both are manifest questions now, so the whole class of "wrote a local key to the server" and "flushed a value the store could not parse" bugs stops being possible by construction. The TypeScript output carries the full definition table — labels, controls, enum members, bounds — so web/src/lib/settingsManifest.ts can be deleted rather than kept in sync: it declared 17 definitions against the contract's 49, with its own two-scope model that does not match the contract's five. make verify-settings-bindings fails when the committed output disagrees with the manifest, wired into CI, so a manifest change cannot merge leaving every client reading stale keys. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(web): add the two-axis quality picker and typed settings hooks Quality becomes one picker over two stored values. The server holds a resolution cap and a bandwidth cap independently, which is what the player has always sent on the wire — useTranscodeQuality.ts has decomposed 1080p-high into {resolution, bitrate} for as long as it has existed. Presets live in the client rather than the contract so retuning what "High" means is a one-line edit here instead of a contract change four codebases have to agree on, and an older server keeps working because it only ever sees the two axes it already understands. A combination no preset covers still gets a truthful label rather than a picker showing the wrong entry: reachable by setting the axes separately through the API, or from a legacy value whose bitrate is off this ladder. Choosing an uncapped preset clears the bitrate rather than storing a sentinel, so "no cap" stays the absence of a value at every layer. Adds hooks over the canonical API alongside the legacy ones rather than replacing them wholesale — a key that is not in the manifest cannot be expressed, because SettingKey is generated from it, and the default for an unset value comes from the generated table rather than a literal at the call site. That last part is what stops the flip-off bug the Apple client carries a hand-written guard for. A test asserts every preset composes values the contract actually accepts, so a preset naming a resolution outside the enum or a bitrate outside the declared bounds fails here rather than 400ing when a user picks it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(settings): resolve catalog playback preferences through the contract catalog/detail.go held the two hardest ladders in the codebase: subtitles resolved across four levels by hand, audio across three, each partially overriding the last through Has* flags. Both now call the canonical resolver, so the precedence lives in the manifest and this file cannot disagree with the contract about which override wins. Adding a scope is a manifest change rather than another branch here. The subtitle track signature stays on its specialized table — it identifies a concrete track rather than expressing a preference, so it is not a setting. Resolution keeps the memoization the old lookups had: the audio resolver still reads once per profile and once per library rather than once per file, which is what kept a many-track audiobook detail page fast. The test that guards it now counts resolver reads instead of GetProfile calls, since the guarantee is about scaling with file count rather than about which method does the reading. Four tests seeded the profile column directly. That column is a migration source now, not a read path, so they seed the canonical value instead — they were passing against storage nothing reads. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(settings): close the unknown-key extension bag keyUsesUserScope returned true for any key the registry did not know, so a client could invent a production setting unilaterally and the server stored it as an unvalidated string. That is how six ui.* settings and five orphan keys reached production untyped, and it is the root enabler the design names. An unknown key is no longer a user setting, so the legacy write path rejects it and the canonical API — which validates every value against its own definition — is the only way to store something new. jellycompat's DisplayPreferences blobs ride the same table under synthetic keys and keep working: they are that subsystem's storage rather than user settings, and they move to dedicated storage in the follow-up rather than being dropped here. Also repoints the DisplayPreferences seed at the canonical resolver. Resolved at profile scope with no device on purpose — Jellyfin clients do not carry Silo's device identity, so a device override leaking into the seed would hand one device's settings to every Jellyfin client on the account. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(settings): enforce viewer quality caps through resolver constraints constraintsFor was the unwired half of the preferences-versus-restrictions seam: it returned nil, so a profile capped at 1080p by policy still resolved its stored 2160p preference at face value through the effective endpoint. The settings routes are mounted inside RequireViewerAccess, so the resolved access scope is already on the request context. Scope.MaxPlaybackQuality holds a literal member of the contract's quality enum ("1080p"/"2160p"), which is exactly what the manifest binds playback.preferred_quality's ceiling to under policy_input "max_playback_quality" — so the wiring is a direct map with no translation table. An empty value means the policy sets no cap, expressed by returning nil so the resolver leaves the preference alone. catalog.metadata_language deliberately stays unconstrained: the manifest notes record that the allowlist draft was circular (the policy input it would bind to is populated from the very preference it would narrow). The handler test covers both halves of the seam: a 2160p preference under a 1080p cap resolves to the cap with constrained:true/ceiling and the authored value reported in stored_value, the stored row itself is not rewritten, and an uncapped viewer gets the preference unchanged with no constraint noise. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(settings): publish user_settings change events Add a user_settings realtime channel so clients learn when a setting changed on another device without polling. The channel is modeled on user_state: non-admin subscribable, per-user addressed envelopes, null snapshot. SettingValuesHandler gains an EventsHub and publishes user_settings.changed after every successful PUT and DELETE on /settings/values/{key}. The payload carries only key, scope and profile_id — never the value. Admins receive every user's user-scoped events, so a value in the payload would leak private settings to admins; interested clients re-fetch over the scoped REST API instead. The payload is always non-empty because an empty Data falls back to a null snapshot in the hub. A nil hub (tests) skips publishing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(settings): sweep expired mutation receipts daily Setting-mutation idempotency receipts were written with an expires_at that nothing enforced, so the table grew forever. Add a hidden daily system task (05:00) that walks every login account, opens its user store, and calls DeleteExpiredSettingMutations. A user whose store fails to open or sweep is logged and skipped so one broken store cannot stall retention for everyone else; the delete is idempotent, so the next run repairs anything missed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(settings): resolve metadata language canonically in access and policy Repoint the last legacy column readers onto canonical contract resolution (settings cutover task A4a): - access.Resolver and policy.ViewerResolver now resolve catalog.metadata_language through settingsresolve (profile scope -> contract default) via a shared access.PreferredMetadataLanguage helper, instead of reading user_profiles.preferred_metadata_language. Resolution is deliberately unconstrained: the policy input this preference feeds is the one a constraint would have to reference, which is circular — see the key's manifest notes. - playback start now resolves playback.audio_language canonically for the profile default instead of reading user_profiles.language, matching the catalog detail path. Series and library override handling is unchanged. - items.go needed no change: it already consumes the resolver-produced scope.PreferredMetadataLanguage. The legacy columns keep their values but are no longer read on these paths; a profile with only a column value now resolves to the contract default, and a stored canonical value wins. Tests pin both directions in access, policy (including scope parity, where the column is now a decoy), and the playback handler. Read cost is one batched store read per resolution, same as the profile-row read it replaces. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(jellycompat): give DisplayPreferences its own table The Jellyfin DisplayPreferences blobs rode the legacy user_settings key/value table under synthetic jellycompat:* keys, which forced the legacy settings API to carry a prefix carve-out in its otherwise-closed unknown-key gate. They are the compat subsystem's storage, not user settings: the contract neither validates nor resolves them. Move them to a dedicated jellycompat_displayprefs table in both backends, keyed by (prefs id, client) per user, with the blob stored as opaque text served back byte-for-byte (deliberately not jsonb, which would re-serialize it). The data-copy migrations — per-user SQLite V16 and a paired SQL + Go goose migration for Postgres — are transactional and harmless to re-run, and both drive their key parsing and row classification from the new internal/jellycompat/displayprefs package so the backends cannot diverge, following the internal/settingsmigrate precedent. A jellycompat:* row that does not parse as a DisplayPrefs key (only ever writable through the removed carve-out) is recorded in user_setting_migration_rejects rather than silently deleted. With the last non-settings tenant gone, the jellycompatSettingPrefix carve-out is deleted: the legacy settings endpoints now refuse jellycompat:* keys like any other unknown key and never surface them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(settings): serve admin user-settings through the canonical API Replace the ten string-registry /admin/users/{id}/settings* and device-settings* routes with the canonical contract surface: one list of every explicit value the target user has stored across all scopes, and set/delete at an explicit scope named in the query string. The admin handlers live on SettingValuesHandler and share the session routes' implementation rather than duplicating it — the same key/scope parsing, identity validation, contract scope allowance, value normalization and mutation-receipt idempotency, factored into keyedScopeFromRequest/completeIdentity and setValueAt/deleteValueAt. The only admin-specific parts are the target user coming from the path, profile and device ids coming from the query (an admin holds no session claim to the user being inspected, so its named profile is checked to exist), and change events attributed to the target user so their clients refresh. The list is a new UserStore read, ListAllSettingValues, implemented in both backends and pinned by the shared storetest conformance suite: the admin surface wants the stored truth (which overrides exist, for a per-row reset affordance), which no resolution-shaped read answers. The ten removed routes are recorded in the pre-lock removals table in docs/architecture/v1-scope.md per the v1 API rules; the web admin device-overrides page moves onto the new surface in the Phase B rewrite inside this same unmerged PR. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(settings): add the cross-platform conformance fixture and its Go and web runners contracts/settings/v1/conformance.json is the spec's named drift gate: 21 hand-authored cases of {keys, stored rows, context, constraints, expected effective value + source}, every one executable against the shipped manifest. They pin the semantics most likely to drift across four resolver implementations: the full resolution ladder (series > library > device > profile > default), an absent identity dropping its scopes, foreign-identity rows never resolving, ceiling caps that report the authored value with constrained:true, the ordered-enum sentinels (auto below every cap, original above), null-on-a-nullable-numeric meaning unbounded and being brought down by a ceiling but ignored by a floor, allowlist falling back to the first allowed member rather than the (possibly forbidden) default, and playback.subtitle_appearance resolving device > profile only with the sparse device object replacing, not merging. Cases may inject a constraint binding onto a copy of a real definition so constraint kinds no shipped definition carries stay testable. The Go runner (internal/settingsresolve/conformance_test.go) resolves each case through the real resolver against the embedded manifest. The web runner (web/src/lib/settingsConformance.test.ts) runs the same cases through a new client-side resolver, web/src/lib/settingsResolve.ts, which mirrors the server's semantics; the TypeScript bindings now carry each definition's ordered flag and constrained_by binding so that resolver derives constraint behavior from the contract instead of hardcoding it. Both runners reject unknown fixture fields — schema drift in the fixture itself is drift — and both refuse a fixture authored against a different manifest revision. The fixture travels with the bindings: make settings-bindings vendors the copy the web runner reads, and make verify-settings-bindings fails CI when that copy goes stale. The Kotlin and Swift copies land together with their runners in the client repos, which will pick their own test-resource paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(settings): review pass over the phase A stack Fixes the eight adversarially-confirmed defects the review of the unpushed phase A stack (40e0f77a..1f2c7fe4) found, each with a test that fails without its fix. Writers left behind by the language cutover (high). |
||
|
rxwatcherandQuick
|
ad122c38c2 | fix(catalog): allocate collision-free subtitle indexes | ||
|
|
7ab393fc3a |
fix(jellycompat): resolve item duration probed-first with runtime fallback (#493)
* fix(jellycompat): resolve item duration probed-first with runtime fallback Jellyfin-protocol clients received no runtime at all for items whose catalog runtime is 0. RunTimeTicks is omitempty, so a zero value is dropped from the JSON entirely rather than sent as 0, and strict clients (Infuse) abandon playback on those items. On the production deployment 5,245 movies have media_items.runtime = 0 while 5,239 of them have a correct probed media_files.duration. Resolve duration at read time the way /api/v1 already does: probed file duration first, catalog runtime as the fallback. The item row is deliberately not backfilled — one item can have several versions of different lengths, so per-file data does not belong there. - scanner: FirstDurationsByContentIDs / FirstDurationsByEpisodeIDs, batched lookups using the same "first live file with duration > 0, ordered by id" rule as the v1 API's contentDurationSeconds. The episode_id IS NULL guard on the content-id query is load-bearing: every episode file carries its series' content_id, so without it a series row would report an episode's duration. - catalog: optional batchDurationFetcher extension on DetailService, following the existing extraFileFetcher pattern so test fakes need no changes. Nil-receiver safe and fail-soft — a failed lookup logs and degrades to the catalog runtime rather than failing the page. - jellycompat: DurationSeconds on upstreamListItem/upstreamEpisode, a shared runtimeTicks resolver, and fillListItemDurations wired into the nine page producers. Fixes the three sites that had no fallback (itemFromList, episodeFromUpstream, HandleSearchHints); the detail and PlaybackInfo paths were already correct. This is additive within the v1 rules — it populates a field that was previously omitted. No field is renamed, removed, retyped, or repurposed. * fix(jellycompat): avoid duplicate duration lookups --------- Co-authored-by: Quick <31828688+Quick104@users.noreply.github.com> |
||
|
|
c52ca7dd7a |
feat(admin): identify compat sessions and Android devices in the live session view (#495)
* feat(admin): identify Android devices by model in live session view Android clients that send a bare default User-Agent (e.g. "Dalvik/2.1.0 (Linux; U; Android 11; AFTKRT Build/RS8180.3729N)") showed up as "Dalvik" in the admin live-session view, which tells an operator nothing about the device. Parse the model code out of the UA (the token between the last ';' and "Build/") and map the Amazon Fire TV family and NVIDIA Shield to product names. Unknown but parseable models fall back to "Android · <MODEL>" instead of "Dalvik"; multi-word models like "Pixel 7" are preserved whole. This is display-only: the session still stores the raw model code in its user agent, and no response field or contract changes. * feat(admin): mark Jellyfin-compat sessions with the JF pill by origin The admin "JF" pill was derived at read time by substring-matching a token list against the client name / user agent. A real Jellyfin client that authenticates through the compat surface but sends a bare User-Agent and no MediaBrowser client name (e.g. a Fire TV app) got no pill, even though it plainly came through the Jellyfin API. Stamp compat origin as immutable identity at session creation and carry it through to the admin view: - ClientInfo.IsCompat is set true in the jellycompat auth path; newSession copies it onto Session.IsJellyfinCompat. - The flag rides the durable RecipeCard (next to the client metadata that already exists so the pill survives reconstruction) and is restored in ReconstructSession, so a server restart keeps the pill. - buildLiveSessionSync -> worker.SessionSync -> a new compat_origin column on playback_sessions_sync (added migration); the reconciler upserts, reloads, and compares it so origin changes still publish and unchanged rows do not churn. - The handler ORs the stored origin with the existing name/UA heuristic, which stays as a fallback for rows written before this column existed. is_jellyfin_client keeps the same name and type on the wire; it is only sourced more accurately. * fix(admin): correct Android device labels --------- Co-authored-by: Quick <31828688+Quick104@users.noreply.github.com> |
||
|
|
148c9291c5 |
feat(web): add Connect Apps settings page for compat sign-in (#488)
* feat(web): add Connect Apps settings page for compat sign-in Jellyfin-protocol clients offer one username box and one password box and never prompt for a profile, so signing in requires `account#Profile` and `password#PIN`. Nothing in the product taught that syntax, and every way of getting it wrong surfaces as the same "invalid username or password", so it became a recurring support burden. Add Settings -> Connect Apps, which states the credentials for the signed-in account rather than describing them in the abstract. The page is segmented by app type: the two formats are never shown at once, each side names the apps it covers, and the compat side is visually distinct so it cannot be mistaken for the normal Silo login. The compat listener's separate address is shown too, since pointing a client at the Silo address fails identically to a bad password. Backend adds GET /api/v1/compat/connect-info, an account-scoped read of the compat listener's enabled flag, public URL, and server name. The admin status endpoint already covers this ground for operators, but it also reports install paths and version provenance, so it stays admin-only; this returns only what a client learns by connecting anyway. It is auth-only and deliberately not profile-scoped, since it describes how to sign in. ConnectInfoForConfig shares the enabled-flag precedence with WebComponentStatusForConfig via compatEnabled, so the two endpoints cannot disagree about whether compat is on. The page declines to display a username it knows cannot work: profile names permit `#` but the resolver splits at the last one, so `alice#Movie #2` parses as account "alice#Movie". Such profiles get an explanation and no copy button instead of a string that fails to authenticate. Part of #432 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(web): harden Connect Apps against misleading sign-in states Review of the first pass found six ways the page could state something untrue. Each one matters more than usual here, because the page exists specifically to stop people guessing at credentials. Report the running listener, not the stored setting. jellyfin_compat.enabled is restart-required and cmd/silo builds the compat server from the boot config alone, so the stored value describes intent. Reporting it promised credentials for a listener that does not exist yet, or claimed the API was off while the running one kept serving. ConnectInfo now returns the boot-time state plus a pending_restart flag, and the page distinguishes "not running yet" from "turned off". The admin status endpoint keeps reporting configured intent, so the two intentionally diverge until a restart. Stop presenting fetch failures as a disabled compat API. React Query clears isLoading on error, so a failed request rendered "the compatibility API is turned off" and sent users to an admin about a setting that was fine. A failed profile list was worse: it fell through to an empty list and offered the bare account name, which silently drops the profile suffix. Both now withhold credentials and say the load failed. Detect accounts that cannot use password login. Compat login is hardwired to the local provider, which rejects accounts with local_password_login_enabled false before checking any password, so SSO and plugin-provisioned accounts can never authenticate. The page told them to type a password anyway; it now says the compat API cannot accept the account. Flag loopback compat addresses. jellyfin_compat.public_url defaults to http://127.0.0.1:8096, which resolves to the client device on the phones and TVs this page names. An untouched default was offered as the exact address to copy; it is now explained instead of presented as usable. Apply the #-in-profile-name guard to the summary list too. The selected-profile field withheld an unusable username while "Every profile at a glance" reintroduced it two sections below. Read only the three settings this endpoint consumes. GetAll on the encrypted repository decrypted every stored secret on each authenticated page view, and an unrelated decryption failure would have silently dropped valid compat overrides. Invalidate the connect-info cache when an admin saves jellyfin_compat.* settings. The address applies without a restart, but the page cached it for five minutes and kept offering the old value for copying. Part of #432 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Quick104
|
f6615ef8d7 | fix(jellycompat): confirm authoritative watch stops | ||
|
Quick104
|
ac1dd99c72 | fix(jellycompat): address scrobble lifecycle review | ||
|
Quick104
|
487dc84829 |
fix(jellycompat): deduplicate playback negotiations
- Replace unstarted negotiations for the same device and item - Apply deduplication atomically across durable store instances |
||
|
Quick104
|
166c5ef32f |
Add reliable Jellycompat watch scrobbling
- Forward start, pause, resume, and stop events with stable media identities - Persist and retry terminal scrobbles across teardown and restart paths - Reject ambiguous playback-report route matches |
||
|
Quick104
|
827843fd21 | fix(jellycompat): omit unspecified color range | ||
|
Quick104
|
b3e2943ccd | fix(jellycompat): preserve video color range | ||
|
|
e56a1b3e03 |
fix(api): throttle api_keys last_used_at writes in auth middleware (#381)
* fix(api): throttle api_keys last_used_at writes in auth middleware Every API key request spawned a goroutine that ran an UPDATE on api_keys, so a key driving HLS segments or a polling integration hit the table with one write per request, and a stalled database could pile those goroutines up without bound. The jellycompat authenticator already guards this same write with a once-per-minute throttle per key; the main middleware was missing it. Bring the two in line. Track the last write per key ID and only launch the update once a minute has passed, with a timeout on the background write. The map is keyed by key ID so it stays bounded. * fix(auth): bound API key last-used throttling --------- Co-authored-by: Quick104 <31828688+Quick104@users.noreply.github.com> |
||
|
|
8044eb84dd |
feat(activity): refine play-method tags and add a Jellyfin-client pill (#387)
* feat(activity): refine play-method tags and add a Jellyfin-client pill
Two related tagging improvements to the admin activity views, squashed:
Split audio transcodes into their own tag. The Play Method summary and
Server Activity popover bucketed every session by its raw play_method,
lumping real video transcodes together with video-copy HLS repackages and
having no separate tag for audio-only transcodes. Classify each session by
the per-stream decisions the backend already reports:
- video re-encoded -> "transcode" (yellow)
- only audio re-encoded -> "audio" (red)
- streams only repackaged -> "remux" (blue, incl. video-copy HLS)
- nothing touched -> "direct" (green)
ordered direct -> remux -> transcode -> audio across the distribution bar,
legend, method filter/sort, the per-row badge, and the Server Activity
stream counts.
Add a Jellyfin-client "JF" pill. Sessions from a Jellyfin-ecosystem client
(Jellyfin Web, Findroid, Swiftfin, Infuse, etc.) get a purple "JF" pill
next to the play-method tag. Detection is UI-only: isJellyfinSession()
positively matches client_name (set from the Jellyfin MediaBrowser auth
header) and then the raw user agent against the known Jellyfin client
tokens, mirroring the server's client-labeling list. The pill is orthogonal
to the method classification — a session can be both "transcode" and JF.
Pure UI/presentation change; no backend behavior changes.
AI-use disclosure: implemented with AI assistance (Claude Code).
* fix(web): cache-control on SPA shell so deploys bust stale UI
The frontend handler served index.html with no cache directives, leaving
freshness to browser/CDN heuristics. A stale index.html at a CDN edge kept
serving old content-hashed bundles, so a client-side hard refresh couldn't
recover — one browser would show the new UI while another showed the old.
Apply the standard SPA cache policy:
- index.html (and SPA-route fallbacks): no-cache + a truncated-SHA-256
ETag, so the shell is cached but revalidated on every load and answers
an unchanged request with a cheap 304.
- /assets/* (Vite content-hashed bundles): public, max-age=31536000,
immutable — cached indefinitely; a new build changes the filename hash,
which busts them automatically.
- other stable-named bundled files (sw.js, icons, fonts): no-cache, so a
changed service worker or icon can't stay stuck in a cache.
Caching is preserved (no no-store anywhere); only the tiny HTML shell is
revalidated, which is what busts a stale UI on deploy.
* fix(activity): compute the method bucket server-side and unify every session surface
Review follow-ups for the play-method tags (PR #387):
- The server now emits effective_play_method (additive field) from the same
per-stream decisions that drive the badges, so all consumers — web, realtime
popover, and the Android/Apple admin views later — agree on the bucket
instead of each client re-reducing raw play_method. Rows with an unknown
play_method (stale rows from older nodes) stay unbucketed rather than being
misreported as audio transcodes off the bare transcode_audio flag; the web
fallback classifier mirrors that and reports "unknown".
- Jellyfin-ecosystem detection moved server-side as is_jellyfin_client, owned
next to the client-labeling rules so the two lists cannot drift; the web
token list is gone. Adds kodi/mpv/delfin/finamp, which reach Silo only
through the Jellyfin compat surface.
- The dashboard stream cards, stats session table, and household streams panel
now use the same classification as the activity page and popover — they
previously showed contradictory tags for the same live session.
- One shared method->label/color table in adminActivityPresentation.ts
replaces the four independent copies (METHOD_META + three switches); the
method column sort now uses the shared cost-order comparator instead of
alphabetical; dead "copy"/"hls" order entries removed and the reachable
"unknown" bucket is styled.
* fix(server): make SPA revalidation RFC-compliant and stop rebuilding the shell per request
Review follow-ups for the SPA cache policy (PR #387):
- Stable-URL bundled files (sw.js, icons, vendor bundles) now carry a content
ETag. The embedded FS has no modtimes, so http.FileServer emits no validator
of its own — no-cache alone forced a full re-download of multi-megabyte
vendor trees on every use because there was nothing to revalidate against.
- Shell and favicon conditional requests go through http.ServeContent, which
implements RFC 9110 If-None-Match semantics (weak comparison, ETag lists).
The previous exact string compare never matched once a fronting proxy
compressed the response and weakened the ETag to W/"...", silently killing
the 304 path in the most common deployment topology.
- The rendered shell (index read + branding render + SHA-256) is cached per
branding snapshot via the new Snapshot.RenderKey instead of being rebuilt on
every request — the 304 revalidation that no-cache makes the common case now
costs two header writes. The misnamed weakContentETag (it emits a strong
validator) is renamed contentETag.
* fix(activity): show the JF pill on every session surface, not just the mobile row
Review comments on PR #387: the JF pill only rendered inside Admin
Activity's sm:hidden mobile row, so the desktop table — and the other
session surfaces that now share the method classification — never
identified Jellyfin-compat sessions.
Extract the pill into a shared JellyfinSessionPill component (renders
nothing for native sessions) and drop it into the Admin Activity desktop
client line, the dashboard stream cards, the household streams panel,
and the stats active-session table.
* fix(playback): sync real encode decisions and client identity for compat transcodes
Review comments on PR #387:
- Jellyfin HLS sessions that copy video and re-encode only audio synced as
full video transcodes: ensureUpstreamPlayback resets transcodeAudio for the
transcode transport method, and the TargetCodecVideo "copy" decision lived
only in TranscodeOpts. A new SessionManager.SetTranscodeStreamDetails
mirrors the actual decisions onto the upstream session when the transcode
starts (local and remote-node paths, via an optional interface so test
fakes are unaffected), so these sessions now bucket as "audio"/"remux".
- Transcode recipe cards now record TranscodeAudio derived from the opts
(only an explicit "copy" leaves audio untouched — empty runs ffmpeg's aac
default), so a session rebuilt after a restart keeps the same bucket.
- Recipe cards carry client name/version/user-agent, and reconstruction
restores them, so the admin client label and the JF pill survive server
restarts; the compat fallback card populates them from the live
MediaBrowser request. Deliberately not projected into stream-token claims,
where a user agent would bloat every stream URL.
* feat(api): capability endpoint for the live-session activity fields
Review comment on PR #387: effective_play_method and is_jellyfin_client are
omitempty, so an independently deployed client cannot distinguish an older
server from a supported one reporting an unknown method or a non-Jellyfin
session. GET /admin/sessions/capabilities advertises both fields plus the
closed bucket vocabulary, following the additive capability-endpoint rule
(same pattern as /collections/capabilities).
* fix(playback): treat empty target audio codec as an AAC re-encode in live state
ffmpeg defaults an empty target audio codec to AAC (appendAudioArgs), and the
new recipe logic already records that as an audio transcode — but the live
native path computed transcodeAudio=false for an empty codec, so the running
stream reported remux until a restart flipped it to audio. Extract the
predicate into playback.TranscodesAudio, share it across the live path, the
recipe card, and the compat mirror, and make appendAudioArgs case-insensitive
so the ffmpeg switch agrees with the predicate for any spelling.
Part of #387 review follow-up.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(jellycompat): re-sync sessions after recording compat encode decisions
ensureUpstreamPlayback flushes the session (compat_start) before
ensureTranscodeSession / startRemoteTranscode record the actual codec
decisions, and that later mutation triggered no sync — so the admin view
showed a video-copy stream as a full video transcode until the periodic
reconciler ran. Trigger syncSessionsNow after the details are recorded
successfully; the helper is shared, so both the local and remote-node
paths are covered.
Part of #387 review follow-up.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Quick104 <31828688+Quick104@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
CoffeeKnyteandGitHub
|
b3722dac58 |
fix(playback): open the listener before sweeping stale transcode dirs (#413)
* fix(playback): run orphaned-transcode cleanup in the background at startup The native and Jellyfin-compat routers swept stale per-session transcode dirs synchronously during NewRouter, before the listener bound. On a slow network filesystem this blocked startup for 80+s (64 leftover dirs on the last deploy), so restart-reconnect clients were turned away and the health check reported the server unhealthy the whole time. Move both sweeps into a background goroutine (StartBackgroundOrphanCleanup) so the listener comes up immediately and the cleanup runs concurrently. The delete logic is unchanged: same active-session snapshot and MaxTokenTTL age-sparing, only later. A package-level mutex serializes concurrent sweeps of the shared transcode root so the two background sweeps can't race on os.RemoveAll. Part of #412 * fix(transcode): background the node boot-time transcode-dir sweep A dedicated transcode node swept leftover transcode dirs synchronously in NewServer, before startStandaloneServer bound its listener. On a slow network filesystem that delete blocked the node from coming online at boot, the same startup-stall class as the main server. Move the sweep into the shared StartBackgroundOrphanCleanup goroutine so the node's listener binds immediately. Backgrounding required an age guard: the sweep previously ran as a full wipe (minAge=0) with an empty active-set, which was only safe because it completed before any request could arrive. Run concurrently that would race a token-carried reconstruct writing into TranscodeDir/<sessionID>, deleting segments a fresh ffmpeg is producing. Passing MaxTokenTTL spares any dir younger than the max token lifetime — exactly the ones a still-valid reconnect could reconstruct — while dirs older than any surviving token (never reconstructable) are still reclaimed. Part of #412 * feat(playback): reclaim orphaned transcode dirs periodically, not just at boot The orphaned-transcode sweep only ran at startup on both the central server and transcode nodes, so it only ever reclaimed dirs left by an ungraceful prior shutdown. During a long uptime the in-memory session reapers delete the dirs of sessions they still track, but a dir whose owning session was dropped without its RemoveAll succeeding becomes an "untracked orphan" with no runtime GC — on a box that runs for weeks these accumulate until the next restart. Add StartPeriodicOrphanCleanup: an immediate background sweep followed by an hourly re-run bound to a lifecycle context. Wire it on all three surfaces — native API and Jellyfin-compat (via deps.AppContext) and the transcode node (via a new Server.StartOrphanSweeper(appCtx), replacing its boot-only sweep). When no context is supplied (tests) it degrades to a single boot-time sweep so no ticker goroutine outlives the caller. The sweep stays age-guarded at MaxTokenTTL, so nothing reconstructable is ever reaped. Because the node sweep now runs during live traffic, it snapshots the live job set (Server.activeSessionIDs) and spares those dirs by id rather than by age alone — a long-lived session that only re-serves already-written segments stops advancing its dir mtime, which age could otherwise misclassify. In integrated mode the native and compat sweeps share one TranscodeDir but each snapshots only its own manager's live set; the resulting cross-manager reap of a >24h idle dir is bounded (rebuilds from token/recipe) and documented at both call sites. Part of #412 |
||
|
|
255b1be89c |
fix(history-import): import Emby favorites (#378)
* fix(history-import): import Emby favorites * fix(history-import): tolerate Emby favorite errors * fix(history-import): count atomic favorite inserts --------- Co-authored-by: Quick104 <31828688+Quick104@users.noreply.github.com> |
||
|
|
28c6ddc237 |
feat(playback): add per-user transcoding controls (#375)
* feat(playback): add per-user transcoding controls * fix(playback): enforce forced video transcode permission * chore: address transcode control review feedback * fix(playback): recheck transcode permission on audio switch --------- Co-authored-by: Quick104 <31828688+Quick104@users.noreply.github.com> |
||
|
|
c1c110e3d2 |
feat(playback): improve web subtitles and track selection (#362)
* fix(player): keep text subtitles in sync across copy-mode restarts and sparse cue windows - Rebase already-loaded cues in place when streamOriginSeconds changes (copy-mode session restart) instead of leaving them offset by the delta. - Stop inferring end-of-input from where a window's cues stop; only the known media duration marks EOF, so a dialogue gap no longer silently ends prefetching for the rest of playback. - Anchor the first window fetch to the intended start position (resume target or pending seek) while the element still reports currentTime=0, and reset coverage on forward seeks past the fetched window. * fix(playback): align encoded transcode start with the declared segment boundary A mid-segment start (resume, seek restart, audio switch) spawned ffmpeg at the raw seek position while labeling its first segment with the grid number, whose synthetic-manifest start is up to one segment earlier. hls.js aligns the first fragment's content to that declared position, shifting the whole session's timeline late by seek mod segment_duration (0-2s): subtitles trail dialogue by a constant per-session offset and progress/resume positions drift by the same amount. Snap the ffmpeg start position down to the segment boundary for encoded sessions so declared and produced timelines match exactly; the player still seeks to the precise requested position. Copy-mode sessions serve ffmpeg's real manifest and keep the raw seek. * fix(api): forward http.Flusher through response-writer middleware wrappers Streamed subtitle extracts (and any progressive response) flush per chunk via an http.Flusher assertion, but none of the status-capturing middleware wrappers implemented Flush, so the assertion failed and cues sat in Go's response buffer until ffmpeg finished. On large remuxes where a 600s window takes 20s+ to demux, captions appeared only when the whole window completed instead of within the first seconds. Give every wrapper a Flush() (satisfies plain assertions, including chi's Compress) and Unwrap() (satisfies http.ResponseController). The jellycompat image-proxy tag rewriter flushes only in passthrough mode since it buffers JSON bodies for rewriting. Regression test asserts the API chain forwards Flush end to end. * feat(player): PGS subtitles honor size, position, and background settings Port the tvOS/iOS bitmap-cue styling to the web player. libpgs now decodes in worker mode but draws to a hidden source canvas on the main thread; a compositor detects cue regions from the frame's alpha channel and re-places them on a visible overlay canvas per the shared subtitle appearance settings: size scale (with the 0.85 authored-size compensation), vertical position preset (dialogue-band cues only — floating signs keep authored placement, matching the Apple implementation), and the background box. Font family, text color, and outline are baked into the source pixels and remain inapplicable. * fix(player): anchor PGS position presets to the text overlay's reference frame The initial port used silo-apple's 30/1080 bottom margin and video-relative lower-third/top anchors; the web text overlay anchors to a 16:9 reference frame with 7%/18% offsets that extends into the letterbox for wide content. Use the same anchors so PGS dialogue lands exactly where SRT text does. * feat(player): size PGS cues to match the text subtitle line height Replace the authored-size ladder (0.85 × font-size ratio) with per-cue text-line matching: the region detector reports the tallest text line inside each cue, and the compositor scales the cue so one line of bitmap text renders at the same pixel height as the SRT overlay's font at the current preset. Authored size differences between discs no longer leak through; upscaling is capped at 2.5× to keep small bitmaps from going blurry. * feat(subtitles): opt-in windowed PGS extraction to cut mid-file load latency PGS extracts always demuxed the source from byte 0, so starting a large remux mid-file meant minutes before the first bitmap cue. The web player now opts in to windowed extraction (?windowed=1&position=&duration=) and re-points libpgs at a fresh window on seeks and near coverage end; ffmpeg input-side -ss with -copyts keeps absolute source timestamps. Without the explicit opt-in the endpoint behaves byte-identically, so Apple/Android and other single-fetch consumers are unaffected. ASS remains unconditionally non-windowed (its header only exists at offset 0). * feat(playback): cache extracted PGS subtitle tracks Every selection of an embedded PGS track re-ran a full ffmpeg extract that demuxes the entire source file from byte 0 — minutes for a large remux — and responses were Cache-Control: no-store, so repeat selections, re-watches, and multiple viewers all paid full price. Add a disk cache for full-track .sup extracts under <transcode_dir>/subtitle-cache, created lazily: - Keyed by source path hash + subtitle stream ordinal + source mtime+size (encoded in the filename), so a replaced source file implicitly invalidates its entries; the source is stat'ed on every lookup. - Cache miss: ffmpeg stdout is teed to the response (first viewer still streams progressively, first-byte latency unchanged) and into a temp file that is fsynced and atomically renamed into the cache on clean ffmpeg exit. Any error — ffmpeg failure, client disconnect, tee write failure, or the source changing mid-extract — discards the temp file, so a partial entry is never served. - Cache hit: served via http.ServeContent (Range support, Content-Length, Last-Modified from the source mtime) with a revalidatable Cache-Control instead of no-store. - Concurrent requests for the same in-flight track run their own uncached extract (mutex + in-flight key set) rather than blocking on another client's connection. - Scan-on-commit LRU eviction under a 2 GiB cap (recency tracked by bumping entry mtime on hit; atime is unreliable under relatime), plus sweep of crash-orphaned .part temp files. - Windowed PGS requests (?windowed=) bypass the cache in both directions: their output covers only a slice of the track. Both the integrated API handler and the standalone proxy subtitle path share the same playback.SubtitleCache.ServeSUPExtract helper. VTT (already windowed and fast) and ASS (small) stay uncached. No API surface change. AI-use disclosure: implemented with Claude Code. * fix(playback): check Close error returns in subtitle cache paths Silence errcheck on the cache-hit defer and the test's simulated disk-full Close. AI-use disclosure: implemented with Claude Code. * feat(playback): warm PGS cache in background and window from cached track Windowed PGS requests bypassed the cache entirely, so every window fetch re-demuxed the multi-GB original file. Now a windowed miss kicks off a detached background warm (full-track extract into the cache, at most 2 concurrent server-wide, coalesced with client-driven fills), and once the entry exists windowed extracts read the 15-80MB cached .sup instead — seeks and re-enables become near-instant after the first load. Verified empirically that ffmpeg preserves absolute PTS when windowing a sup input. * feat(player): hold playback while PGS subtitle cues load When a PGS track is enabled (or a seek lands outside the fetched window), extraction takes seconds and dialogue could play unsubtitled. The player now pauses until the renderer's parsed data covers the playhead — tracked via libpgs' parsed-timestamp watermark, the exact predicate it renders by — showing a "Loading subtitles…" indicator after 500ms. User play/pause always wins over the hold, a 20s safety timeout prevents stranding playback, and background prefetch never pauses. If future libpgs versions reshape the observed internals the hook degrades to the old play-through behavior. * perf(player): shrink uncached PGS window to 600s Draining a windowed extract from the source reads the full interleaved container across the window (~1GB per 100s of remux on measured hardware); a 3600s window cost ~12GB of reads per fetch while cold. Once the server cache is warm a window costs milliseconds regardless of size, so smaller windows only add trivially cheap re-fetches. * feat(subtitles): burn in PGS/bitmap subtitles for the web player The web player rendered PGS client-side via libpgs, which required extracting the .sup track — a cold ffmpeg demux that took seconds even windowed, since c:s copy still reads the whole interleaved container up to the playhead. Every other server (Plex, Jellyfin default, Emby) burns image subtitles into the video instead, and that is the only path with no per-seek extraction cost. Selecting a bitmap subtitle (PGS/DVD/DVB) now restarts the transcode with subtitle_burn_in at the current aligned position, reusing the same restart machinery as an audio/quality switch so the segment-boundary timeline alignment holds. The server composites the decoded subtitle onto the video with an overlay filter_complex graph (libass's subtitles= filter is text-only); overlay runs at native resolution before any target scaling, and hardware pipelines round-trip through CPU like the text path. Burn-in forces a video encode, so copy-video recipes are upgraded to h264 both client- and server-side. Text subtitles keep the instant, styled, client-side path. The .sup streaming endpoints, cache, and windowing are retained for the Apple client, which renders PGS natively. The now-dead web PGS stack (usePGSSubtitles, pgsPlacement, libpgs dep) is removed. Tradeoff: bitmap subtitles no longer honor web appearance settings (baked into the video) and toggling one restarts the transcode (~1-2s buffering), matching Plex behavior. * fix(player): rebuild text subtitle track when turning off PGS burn-in Selecting an SRT track that turned off bitmap burn-in restarted the transcode, and the client TextTrack built in that same moment was orphaned when the <video> element reloaded, so the subtitles never rendered (and a seek could not recover the dead track). Rebuild the text track once the new stream settles, gated on the burn-in-off transition so quality/audio switches and copy-mode seek restarts keep their subtitles without a needless re-extract. * fix(player): render web subtitles behind the control HUD The text subtitle overlay sat at z-20, above the controls layer (z-10), so cues painted over the bottom HUD and cluttered the control bar. Drop it to z-[5] — above the video, below the controls — so the HUD paints over the cues while it is visible. When controls are hidden the whole controls layer is opacity-0, so cues remain fully visible. * feat(player): lift web subtitles above the control bar while it's visible Rather than hiding bottom-anchored cues behind the HUD, raise them just above the control bar (measured height + a small gap) whenever the bar is visible in the foreground player, then settle them back when it hides. The bar is a roughly fixed pixel height while the cue offset scales with the player, so the bar is measured via ResizeObserver rather than hardcoded. Top-anchored cues never collide with the bottom HUD, so they stay put. z-[5] is retained as a safety so any residual overlap tucks behind the bar. * fix(player): coalesce same-tick transcode restarts into one dispatch Starting playback with a persisted bitmap subtitle fired transcode/start twice within milliseconds: the auto-start effect dispatched before subtitle auto-selection restored the burn-in, whose effect then forced a second start. The first request was already on the wire (no abort signal was passed to fetch), so the server spawned an ffmpeg only to kill it for the second start — visible in production as an ffmpeg exit error ~1ms after every such session start, and slowing time to first frame. Defer the network dispatch by one macrotask so back-to-back restart calls in a tick collapse into a single request carrying the final parameters; state updates stay synchronous. Pass the abort signal into playerFetch so a superseded in-flight request is actually cancelled, and drop any deferred dispatch on unmount so a stray transcode/start cannot land after the session's exit DELETE. * fix(catalog): resolve effective subtitle defaults for movie item details Movie pre-play subtitle selectors were missing the effective defaults (including per-item overrides saved from a previous play) that episodes and watch payloads already resolve. Extract applyToItemDetail/ applyToWatchDetail helpers and apply defaults for movies in buildMediaItemDetail. The SubtitlesPopover now also eagerly loads downloaded subtitles when the saved preference points at one so the closed trigger's Auto summary reflects the override. * feat(player): scale subtitle font size with the rendered video Replace fixed rem font sizes with px values defined at a 720px 16:9 reference height, scaled proportionally with the actually-rendered video (object-fit: contain) so subtitles keep the same relative size as the window grows or shrinks, with a 12px legibility floor. Rename useSubtitlePositionStyle to useSubtitleLayout, returning both the position style and the font scale, and add unit tests for the appearance helpers. * fix(player): satisfy strict index checks in transcode quality test * feat(player): let the pre-play Auto option clear the saved subtitle override A manual in-player subtitle selection persists as an 'always' override for that movie/series, but nothing in the UI could undo it — auto selection stayed pinned to the chosen track forever. Choosing 'Auto' in the pre-play subtitles popover now also deletes the stored preference (movie content ID / episode series ID) and invalidates item details so profile-level auto selection applies again. * feat(player): persist pre-play subtitle selections as the item override Choosing a track (or Off) in the pre-play subtitles popover only lived in component state: it applied to that playback session but vanished on returning to the detail page. Persist it through PUT /subtitle-prefs — the same 'always'/'off' override a manual in-player selection saves — keyed by movie content ID or episode series ID, and invalidate item details so the effective defaults reflect it immediately. * feat(ui): show the saved subtitle override and richer pre-play pill summaries A stored per-item override displayed as 'Auto: <language>', hiding both that an override exists and which track it is. The pre-play subtitle pill now shows the resolved track directly (name with (SDH)/(Forced) markers plus format, skipping markers the name already carries), the matching list row gets the checkmark instead of Auto, and the Auto row reads 'Reset to profile defaults'. Subtitle, audio, edition, and version pill summaries also truncate much later (max-w-44/sm:max-w-64). * fix(player): recover text subtitles from stream reloads and failed window fetches Three failure modes could silently freeze or stop web text subtitles: - A stream restart (seek-triggered transcode restart, quality/audio switch) reloads the <video> element and can orphan the programmatic TextTrack — cuechange stops firing and the last cue freezes on screen. Only the PGS-burn-in-off transition rebuilt the track. Now every settled stream URL change bumps the generation, and the rebuild carries loaded cues (converted back to source time) and window coverage over so it costs no refetch. - The sliding-window fetcher committed windowEnd before the fetch ran, so a failed or hung window counted as covered and was never retried — subtitles silently stopped for up to 10 minutes. Coverage now commits only after the window streams in fully; failures leave the range uncovered and retry after a 5s backoff. - A hung extraction (one fetch in flight at a time, no deadline) blocked every future window for the session. Reads now arm a 30s stall timer that aborts a response which stops delivering chunks; slow-but- progressing streams keep resetting it. Diagnosed from a session where ffmpeg took 69s to stream one subtitle window and a transcode restart landed mid-fetch, freezing the active cue. * fix(playback): keep subtitle selections stable across file changes * fix(web): clarify subtitle labels and positioning * fix(player): hide HUD when pointer leaves * fix(web): tidy subtitle track badges * fix(playback): remap audio tracks across file versions * fix(web): tidy audio track labels * docs(playback): clarify bitmap subtitle appearance * fix(playback): preserve selection state on restart * fix(http): preserve response state across flushes * fix(web): preserve pending quality for burn-in * fix(playback): preserve subtitle inventory identity --------- Co-authored-by: Quick <31828688+Quick104@users.noreply.github.com> |
||
|
|
8b9149a789 |
feat(catalog): remove smart collection item limit cap (#340)
* feat(catalog): remove smart collection item limit cap MaxSmartCollectionItemLimit silently clamped every smart collection to 500 items, and an unset limit defaulted to 100. On servers with large libraries this truncates collections with no indication to the admin — a per-language smart collection (original_language is fr) has 10,625 real matches and showed under 1% of them. Remove the cap and make an unset limit resolve the full result set: the default becomes a large sentinel so every consumer's int plumbing (SQL LIMIT) keeps working. An explicit positive limit in the query definition is still honored as-is — membership size is an admin decision, and every read path that serves collection items either paginates in SQL or scales with the actual collection size. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(jellycompat): page smart collection browse in SQL The Jellyfin BoxSet children path resolved a smart (live-query) collection's entire membership and sliced one page locally. With the item cap removed (smart collection membership is now uncapped, an admin decision), a large collection browsed from a Jellyfin client materialized tens of thousands of rows per request — latency and memory scaling with collection size. For the default browse (no explicit SortBy), page the smart query directly in SQL via QueryExecutor.PreviewPage(offset, limit), hydrating only the requested page. Per-request work is now proportional to page size, not collection size. TotalRecordCount still reports true membership (the count query caps at the query's item limit), and past-the-end pages preserve the real total. Shared query prep is extracted into prepareSmartCollectionQuery, used by both the new paged path and the existing full-resolution path (still used by the explicit-sort allowlist branch, whose output is already SQL-paginated). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: rxwatcher <rxwatcher@users.noreply.github.com> |
||
|
CoffeeKnyteandGitHub
|
a2ef26bece |
perf: root-cause fixes for endpoints still slow after #292 (NextUp, series badges, resume tail, subtitle fonts) (#350)
* docs(plans): root-cause analysis for endpoints still slow after PR #292 Five endpoint groups stayed slow after the home/Continue Watching/Latest latency work shipped: Resume (110s p95), NextUp (17s p95), Latest (17s), /Items, and the home sections routes. The caps and caches from PR #292 are live in the deployed binary; they bounded how many rows the loops touch but not what each underlying query costs. Documents the four confirmed root causes (4.3M stale completed-with-position progress rows + missing resume index, unbounded next-up anchor scan, per-episode series rollup fanout, two index-starved history/scanner paths) with live EXPLAIN ANALYZE measurements and the fix plan implemented by the follow-up commits. AI-use disclosure: analysis and doc produced with AI (Claude) assistance. * perf(catalog): bound the global next-up anchor scan to recent completions The completed_episodes CTE in buildListNextUpQuery derived per-series anchors from the profile's ENTIRE completed history — DISTINCT ON over 233k rows joined to episodes for the worst bulk-import profile, then a per-series LATERAL that scans every episode of a fully-watched series before yielding nothing. 648 slow executions in a 19h window, 44.7s worst; this drove /Shows/NextUp (17.1s p95) and the next-up injection on the native home sections aggregate. Global queries now derive anchors from the profile's nextUpAnchorMaxRows (500) most recent completed rows — an ordered index walk on idx_uwp_profile_completed, with the hidden-items exclusion and date cutoff applied inside the bounded scan so hidden/old rows never consume the anchor budget. A next-up rail surfaces ~24 series; the 500 most recent completions cover every series that can realistically rank on it. Series-scoped calls (the show-detail tile) keep the unbounded shape: they must anchor on the series' last completed episode no matter how long ago it was watched, and are naturally bounded by one series. Measured on the live worst-case profile with the exact generated SQL: 44.7s worst / ~2.6s avg before; 10ms after (together with the one-time stale-resume-point data repair applied directly to the deployment DB — see docs/superpowers/plans/2026-07-06-slow-endpoint-root-causes.md). AI-use disclosure: implemented with AI (Claude) assistance. * perf(jellycompat,userstore): aggregate series watch-state rollup in SQL The series Played/UnplayedItemCount badge on list rails (per-library Latest, library browse, search results) and series detail pages was computed by materializing EVERY episode of every series on the page (episodeRepo.ListBySeriesIDs) and then batching per-episode progress+history lookups in 500-id chunks. A 50-series page of an episode-heavy library (Sports) expanded to 32,467 episode rows and ~65 sequential queries — measured 17-18s per /Items/Latest request, and PR #292's cached Latest fast path pays it on every response for series libraries. The same fanout made /Items?searchTerm=... slow whenever the result set was mostly series (Meilisearch itself answers in milliseconds). New optional store capability userstore.SeriesEpisodeRollupStore, implemented by PostgresUserStore as one GROUP BY e.series_id aggregate with semantics identical to the chunked path (episode availability via episode_libraries, hidden-items visibility on progress rows, completed-history fold, in-progress = not watched with position > 0 — verified value-for-value against the old semantics on a real 1,586-episode series). enrichSeriesListUserData and enrichDetailUserData use it when present; SQLite-backed stores and rollup query failures keep the existing chunked path as fallback. catalog.SeasonUserDataFromCounts pins the counts-to-DTO mapping to EpisodeRollupUserData. Measured on the live worst-case profile against the real 50-series Sports Latest page: ~17s of chunked round-trips before, 119ms in one query after. Part of docs/superpowers/plans/2026-07-06-slow-endpoint-root-causes.md. AI-use disclosure: implemented with AI (Claude) assistance. * perf(catalog): bound superseded-episode completed walk to recent history The Resume / Continue Watching superseded-episode filter loaded a profile's *entire* completed history into memory on every request that contained an in-progress episode: CompletedProgressSnapshots paged user_watch_progress WHERE completed=TRUE with no upper bound. The 2026-07-06 slow-query comparison showed this surviving as a 60-116s Resume tail even after the in-progress index landed live, because the 4.3M zeroed Plex-import rows are still completed=TRUE and were re-walked every load. A completed episode can only supersede an in-progress one it was finished more recently than (the query gates on done_progress.updated_at > ip_progress.updated_at), so only completed rows newer than the oldest in-progress entry can matter. Compute that cutoff in SupersededEpisodeProgressIDs and pass it to CompletedProgressSnapshots, which — since the completed listing is ordered updated_at DESC — stops paging as soon as it crosses the cutoff. Import-heavy profiles whose back-catalogue predates their current in-progress items now stop on the first page instead of paging hundreds of thousands of irrelevant rows. Correctness is unchanged: no relevant superseding row is excluded. * perf(catalog): hard-cap superseded-episode completed walk at 5 pages The updated_at cutoff added in the previous commit bounds the completed walk on the relevance axis, but a very old in-progress entry sitting behind a large volume of newer completions could still page deep. Add a 5-page (2,500-row) hard backstop on top of the cutoff: normal profiles still stop on page one via the cutoff, and only the adversarial tail hits the cap. When it engages the tail of the completed set goes unscanned, so a superseded episode could momentarily survive on Continue Watching — we log a warning when that happens (with profile_id + rows scanned) rather than mis-filter silently, and it self-corrects once the stale in-progress entry ages out of the scanned window. * perf(playback): extract subtitle fonts in a single ffmpeg pass Embedded ASS/SSA font extraction spawned one ffmpeg process per font attachment, each re-opening the (usually CephFS-backed) media file. Anime releases carry 15-47 fonts, so the per-spawn file-open cost dominated and pushed GET /api/v1/stream/{sid}/subtitles/{track}/fonts to a 17-60 s plateau (p95 ~33 s in the live logs). Collapse the N spawns into one ffmpeg invocation that dumps every attachment to a temp dir (-dump_attachment:idx path ... -i file -map 0:t? -c copy), then read the files back. The file is opened once instead of N times, taking p95 from ~30 s to ~1-2 s with no change to output. Safety is preserved. The 32-attachment / 32 MiB caps still apply: attachment size is stat'd before read so an over-limit font never enters memory, and a watchdog polls the dump dir and kills ffmpeg if its on-disk output crosses the cap -- restoring the hard bound the old pipe-per-attachment reader enforced by killing at maxBytes+1, so a container with oversized "font" attachments can't fill the disk. Part of the slow-endpoint follow-up; see slow-query-analysis/subtitle-fonts-extraction-findings.md. * fix(review): report enforced font-byte cap; correct doc subtitle scope Address PR #350 review: - dumpFontAttachments reported the maxSubtitleFontBytes package constant in both over-limit errors instead of the maxBytes argument the caller passed, so the message misstated the enforced bound whenever a different cap was in effect (as the tests use). Interpolate maxBytes in both messages. - The root-cause plan claimed subtitle extraction was 'out of scope' while the branch actually optimizes /subtitles/{track}/fonts. Scope the out-of-scope note to subtitle *track* conversion and record the fonts single-pass work as deliverable 5. |
||
|
|
203a18ae83 |
feat(observability): OpenTelemetry logs+traces with secret redaction and slog standardization (#290)
* feat(observability): OpenTelemetry logs+traces with secret redaction Part of #265. Adds opt-in OpenTelemetry (logs + traces) alongside the existing stderr + opslog pipeline, plus secret redaction on all sinks. Default-off: with no OTEL_* / SILO_OTEL_ENABLED config, behavior is unchanged. Bootstrap (internal/telemetry): - Setup() builds one shared resource, a TracerProvider (parent-based trace-id ratio sampler), a LoggerProvider, and the W3C TraceContext+Baggage propagator from env. It installs NO MeterProvider — metrics stay on Prometheus, and the built-in no-op global MeterProvider keeps the trace instrumentation libs from double-emitting. Shutdown is deferred with a flush timeout. - Logs are bridged via otelslog fan-out (slog.MultiHandler), level-gated by the shared LevelVar and best-effort so a failing collector can't break the console or DB branches. stderr + opslog stay untouched. Secret redaction (internal/logredact): - A slog.Handler masks secret-keyed attributes (password, token, api_key, authorization, cookie, ...) — including .With-bound attrs, nested groups, secret-keyed group subtrees, and values behind a LogValuer — on the console and OTLP sinks, with a no-op fast path when a record has no secret keys. opslog.shouldRedact delegates to logredact.SecretKey so all sinks share one marker list. Rotation is infra-managed (no custom file sink): container runtime for stderr, collector/backend for OTLP, opslog partition-pruning for the DB. Documented in docs/architecture/observability.md. Verification: go build ./..., go vet, gofmt -l — clean; go test ./internal/telemetry/ ./internal/logredact/ -race pass. AI-use disclosure: implemented with AI assistance (Claude Code), including adversarial reviews that hardened the bootstrap and fixed two redaction leak paths; reviewed by the author. * refactor(observability): slog context+component sweep, sloglint gate (phase 3) Part of #265. Builds on the OTel bootstrap + redaction commit. Standardizes every log call site onto the context-carrying slog variants so records correlate with the active OpenTelemetry trace, and locks the standard in with a machine gate so future code (human- or AI-authored) can't drift back. - Call-site sweep: converted the remaining slog.<Level>(...) calls to the slog.<Level>Context(ctx, ...) form wherever a context.Context is in scope (background/init calls with no ctx are left as-is), across 183 files. Applied via a type-aware AST codemod. Log levels and message strings are preserved verbatim; a component attr (canonical per-package name) is added to direct package-level slog calls. Bound-logger calls keep their existing .With bindings. The main.go and telemetry package conversions rode with their file in the previous commit to keep each file within a single commit. - Enforcement (.golangci.yml): enable sloglint with context=scope, static-msg, key-naming-case=snake, no-mixed-args. After the sweep all four report zero violations repo-wide (tests included), so make lint / CI now blocks any regression to the non-context form. The gate ships with the sweep because it cannot be green until the legacy sites are converted. Metrics remain on Prometheus; no behavior change to /metrics or Grafana. Verification: go build ./..., go vet ./..., gofmt -l — clean; sloglint (all 4 rules) 0 violations repo-wide; log levels verified unchanged. AI-use disclosure: implemented with AI assistance (Claude Code), including the codemod; reviewed by the author. * fix(observability): honor per-signal OTLP protocol and secret WithGroup names Two Codex review findings on PR #290: - telemetry: OTEL_EXPORTER_OTLP_{TRACES,LOGS}_PROTOCOL now override the generic OTEL_EXPORTER_OTLP_PROTOCOL per signal, so mixed collector setups (e.g. HTTP logs + gRPC traces) build the right exporter. - logredact: entering a group whose name is secret-bearing (e.g. WithGroup("authorization")) now masks every leaf in that subtree, matching how slog.Group("authorization", ...) is masked as a whole. * fix(observability): address review feedback on telemetry bootstrap - Telemetry setup failure no longer kills boot: Setup returns usable no-op providers alongside the error and main logs and continues with telemetry disabled, honoring the best-effort contract. - Honor OTEL_TRACES_SAMPLER (always_on/off, traceidratio, parentbased_* variants); unsupported values fall back to parentbased_traceidratio. - Attach node identity as semconv service.instance.id instead of the non-semconv node.name. - Rename opslog retention-scope log attrs to target_component/target_level so they no longer collide with the canonical component routing key, and tag those lines with component=opslog. - Fix stale levelGated comment casing; use WarnContext in the telemetry shutdown defer; document the LogValuer double-resolve on the redaction slow path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Quick <31828688+Quick104@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
76b10b822e |
feat(jellycompat): surface audio profile and spatial format on media streams (#344)
* feat(scanner): capture audio stream profile metadata Store ffprobe audio stream profiles in the existing audio track metadata JSON. This mirrors the video track profile field and is additive for clients that want to display or inspect codec profile details. * feat(catalogseed): carry audio profile in seed export records Keep AudioTrackRecord in parity with VideoTrackRecord, which already mirrors the video profile field, so seed exports do not drop the new audio profile metadata. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(jellycompat): surface audio profile and spatial format on media streams Set MediaStream.Profile from the captured ffprobe audio profile and derive AudioSpatialFormat (DolbyAtmos / DTSX) from it, matching Jellyfin's case-insensitive substring semantics. Audio display titles now prefer the profile over the codec name (except the AAC "LC" profile), matching Jellyfin, so DTS-HD MA and Atmos tracks read correctly in clients like Infuse. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Silo Contributor <silo-contributor@example.invalid> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
4e03f4b807 |
fix(playback): last-write-wins progress and DV P7 RPU strip on remux (#334)
* fix(playback): last-write-wins progress and DV P7 RPU strip on remux Progress: UpdateProgress (the live playback-session path) clamped position_seconds to GREATEST(new, old), so a deliberate backward seek could never persist — "rewind and stop" resumed at the stale later position on every client. Position is now last-write-wins, matching the /sync/progress path that was always unconditional. The completed latch and rewatch re-entry semantics are unchanged. Remux: profile 7 Dolby Vision remuxes drop the enhancement-layer track (-map 0:v:0 keeps only the base layer) but previously left the dangling dual-layer RPUs on the BL — broken metadata that a DV-honoring display can mis-render. Remuxes of P7 files now strip DV RPUs via the dovi_rpu bitstream filter, yielding a clean HDR10 stream (the same fallback presentation the Apple client's P7 HDR10 toggle produces). Profile 8 RPUs are kept: the BL is self-contained and DV clients render it. Adds MediaFile.PrimaryDVProfile() and threads the profile through ServeRemux callers; the proxy path (no track metadata in claims) keeps prior behavior. True P7->8.1 DV conversion needs dovi_tool alongside FFmpeg (the dovi_rpu bsf only strips/recompresses); the remux plumbing now carries the DV profile so that can slot in later. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(userdb): apply last-write-wins progress to the SQLite backend too Review follow-up (P2): the LWW change only covered pgstore; the SQLite userdb UpdateProgress kept the MAX clamp, so rewind-and-stop still resumed at the stale later position for sqlite-backed installs. The conflict clause now matches Postgres (position last-write-wins, completed latch and rewatch re-entry unchanged), with a backward-seek regression test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(playback): harden LWW progress and DV RPU strip from review - Probe ffmpeg for the dovi_rpu bitstream filter once per process and fall back to a no-strip remux (the pre-existing behavior) when it is missing: on pre-7.1 ffmpeg the unknown filter aborted the process, turning every Dolby Vision profile 7 remux into a hard playback failure. - Skip zero-position heartbeats in persistProgress, mirroring the stop path and the jellycompat report path. Under last-write-wins an early zero heartbeat (e.g. before the client seeks to its resume point) would wipe the stored resume position; GREATEST previously masked this. - Carry the DV profile in stream token claims (dvp, omitempty) so standalone proxy nodes strip profile 7 RPUs the same way integrated mode does. Old tokens decode as 0 and keep prior behavior. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: rxwatcher <rxwatcher@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Quick <31828688+Quick104@users.noreply.github.com> |
||
|
yammes08andGitHub
|
4a76016015 |
fix(jellycompat): tolerate item-id mediaSourceId on direct-play streams (#313)
Native clients that call PlaybackInfo, reuse the returned PlaySessionId on the
/Videos/{id}/stream?static=true request, but send the *item* id as mediaSourceId
(per the Jellyfin MediaSource.Id == Item.Id convention) rather than Silo's
fileID-based source id got a 400 "Media source is required" and could not
direct-play at all.
The PlaySessionId branch of resolvePlaybackRoute returned findMediaSource's nil
result directly, with no fallback -- unlike the FindByRoute and
createStaticPlaySession paths. Because resolution returned a nil error,
HandleVideoStream's Static fallback never fired either, so every direct-play
request 400'd.
Fall back to the session's primary source only when mediaSourceId is empty or
equals the route item id (the MediaSource.Id == Item.Id convention), matching
Jellyfin's StreamingHelpers. Any other unmatched id -- a stale/foreign id, or a
wrong version on a multi-version item -- stays rejected so it can't silently
stream the wrong file. Exact source-id matches are unchanged.
|
||
|
|
e140bd9424 |
feat(metadata,scanner): trailers and extras for movies and series (#322)
* feat(metadata,scanner): trailers and extras for movies and series Remote provider videos (TMDB trailers/teasers/featurettes/...) are fetched through the unified match/refresh pipeline into the new item_videos table, filtered per-library via media_folders.trailer_kinds, merged across providers with site/provider dedup, and lockable via FieldVideos. The movie scanner stops discarding supplemental directories (Trailers/, Featurettes/, Behind The Scenes/, ...) and classifies them — plus Jellyfin-style filename suffixes (-trailer, -behindthescenes, ...) and series-root supplemental dirs — into the new media_extras entity backed by ordinary media_files rows (extra_id ownership, content_id/episode_id NULL so existing version/matching queries stay structurally blind to extras). Series Extras/SxxExx season-0 mapping is unchanged. Extras are playable watch targets via a GetWatchDetail fallback tier (episodes precedent), with contentid.ForLocal minting stable ids. API: ItemDetail gains additive videos/extras arrays (single + batch parity); library settings expose trailer_kinds. jellycompat now populates RemoteTrailers, LocalTrailerCount/SpecialFeatureCount, and serves real /LocalTrailers + /SpecialFeatures items playable through PlaybackInfo. Requires silo-plugin-sdk v0.9.0 (VideoRecord) before go.mod can bump; builds locally via go.work against the SDK feat/metadata-videos branch. Part of trailers/extras capability work. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(web): trailers and extras sections, library trailer-kinds setting TrailersSection (YouTube thumbnails + youtube-nocookie modal) and ExtrasSection (plays extras through the standard watch controller) on movie and series detail pages; admin library form gains a trailer-kinds allow-list synced with the server default (all provider kinds), now also honored on library create. Part of trailers/extras capability work. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(scanner): scan extra_id in scanMediaFiles; review cleanups scanMediaFiles (the plural row scanner behind GetByContentID/GetByFolder/ GetByExtraID and 20+ other queries) was missing the scan destination for the new extra_id column, which would have failed every media-file read at runtime with a column/destination count mismatch. Also: extend the batch equivalence test to seed item_videos/media_extras so the new videos/extras prefetch wiring is actually proven; drop the one-off pgxRows interface for the repo-wide pgx.Rows convention; reuse formatClock instead of a third duration formatter in ExtrasSection. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(deps): bump silo-plugin-sdk to v0.9.0 for VideoRecord Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(matching): exclude extras files from match queues and bulk content linking Dev verification caught extras media_files rows (content_id NULL by design) being swept into the movie/series match queues and the root-claim bulk relink: a '-featurette' suffix extra was matched onto its parent as a version, and a Trailers/ file minted a spurious local skeleton item that shadowed the extra's watch target. Add 'extra_id IS NULL' to the queue eligibility conditions, root/group claim relinks, observed-root content assignment, and the admin unmatched-files listing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(playback): authorize local extras files through their parent item Dev verification: playback/start (and the shared MediaFileAuthorizer used by markers/subtitles/ebook reader) resolved file ownership only via episode_id/content_id, so extras files (extra_id only) 404ed. Add an ExtraLookup tier that resolves media_extras and gates on the parent item's access, mirroring the episode->series pattern. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(catalog): resolve local extras through GetItemDetail for compat playback jellycompat PlaybackInfo (and any per-item consumer resolving arbitrary content ids) goes through GetItemDetail, which lacked the extras tier that GetWatchDetail has — so Jellyfin clients got zero MediaSources for extras. Add buildExtraItemDetail (minimal detail + ordinary playback surface, parent-gated access) as the fourth resolution tier, and map the extra type to Jellyfin's Video kind. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(web): allow youtube-nocookie embeds in CSP; trailer modal a11y The frontend CSP's frame-src blocked the trailer modal's youtube-nocookie.com iframe (found on dev verification). Also add the missing sr-only DialogDescription and drop the redundant allowFullScreen attribute. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: address PR review findings for trailers/extras - Extras watch/item detail no longer stamp SeriesID/SeriesTitle for movie-owned extras (players key episodic post-roll flows off series_id); series-owned extras keep them (Codex). - processExtraFiles resolves the parent and upserts media_extras before the unchanged fast-path, and the fast-path now also compares mtime, so rematched parents / reclassified kinds / same-size replacements converge (Codex + CodeRabbit). - media_files upsert clears content/episode linkage atomically when extra_id is set (ownership mutual exclusion in one statement); the now-redundant MarkFileAsExtra helper is removed (CodeRabbit). - ScanFile's extras branch runs syncPresentLibraryState + reconcileLibraryMemberships so converting a primary file to an extra cleans stale library membership immediately (CodeRabbit). - media_extras migration adds the media_files FK as NOT VALID + VALIDATE to avoid a full-scan exclusive lock on large tables (CodeRabbit). - trailer_kinds input is trimmed/lowercased/deduped and unknown values are dropped instead of silently widening the allow-list to 'other' (CodeRabbit). - Extras authorization branches match the episode branch's posture: unconfigured lookup is a config error, nil extra is a 404 (CodeRabbit). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
42602b7896 |
feat(policy): access groups + embedded OPA policy engine with decision audit log (#282)
* docs(policy): add OPA policy engine design spec and implementation plan Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * build(deps): add OPA v1.18.2 SDK for the policy engine Pulls github.com/open-policy-agent/opa v1.18.2 (policy engine core for the upcoming internal/policy subsystem) and the transitive upgrades go mod tidy applied (otel 1.44, grpc 1.81.1, prometheus/common 0.67.5). Full build verified. Part of #272 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(policy): add OPA engine core, vendor scope policy, and parity suite New internal/policy package (dead code — nothing wires into request paths yet): prepared-query Engine with 25ms eval timeout and fail-closed decode, typed PDP.ResolveViewerScope, go:embed vendor bundle, capabilities lockdown for future admin-authored Rego, and vendor scope.rego reproducing access.Resolver.Resolve (library intersection, disabled-library handling, quality/rating ceilings) with a narrowing-only silo_custom.scope.override extension hook. Parity proven by 1368 dual-execution subtests against the real access.Resolver, including the nil-vs-empty AllowedLibraryIDs battery and quality/rating variation; rank tables are test-pinned to internal/access. Rego unit tests run via opa/v1/tester inside go test. Bench: ~106µs/op per scope decision incl. input marshaling. Also restores the OPA requirement to go.mod (the earlier deps commit ran go mod tidy before any import existed, so tidy dropped it). Implementation drafted by Codex (GPT-5.5) via codex exec; reviewed, corrected (quality.allowed raw-file-rank divergence), and verified here. Part of #272 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(policy): add policy document store, foundation schema, and compile-check policy_foundation migration: policy_documents (one enabled doc per domain via partial unique index — two enabled docs would define override twice and conflict at eval), immutable policy_document_versions, single-row policy_generation counter, and the partitioned policy_decisions log table (daily range partitions, no FK, denial partial index). PolicyStore: transactional version numbering (FOR UPDATE), activation that verifies compiled_ok and bumps the generation in the same tx, enable/disable with typed ErrDomainAlreadyEnabled, and a delete guard for documents with an active version. CompileCheck sandboxes admin Rego: locked capabilities (no http.send/net.*/opa.runtime), enforced silo_custom.<domain> package path, vendor+stub layering, 2s budget, structured row/col errors. Engine gains NewEngineWithCustom / NewEngineFromStore with WARN-and-skip for invalid custom rows. DB-backed tests verified against a migrated Postgres (concurrent version numbering, atomic generation bumps, activation guards). Implementation drafted by Codex (GPT-5.5) via codex exec; reviewed and verified here (domain constants extracted). Part of #272 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(policy): add policy System lifecycle with hot reload and cross-node invalidation policy.System owns one long-lived Engine and reloads it in place when policy documents change: EventPolicyChanged on the existing ChannelAdmin bus (new cache event constant) plus a 60s generation-poll fallback for Redis-less deployments, with a generation-consistent snapshot read. Vendor compile failure is startup-fatal; store/custom failures degrade to vendor-only and the poll loop heals them; runtime reload failures keep the last known-good engine. NotifyChanged gives the future admin handlers synchronous local reload + cross-node publish. Wiring: constructed in integrated/api modes only, PolicySystem field on api.Dependencies (unused by routes yet), policy.eval_timeout_ms setting (hot-reloaded via configWatcher.OnChange; default 25ms). Verified by a full server boot smoke and DB-backed convergence tests (event + poll paths, degraded boot, last-known-good). Implementation drafted by Codex (GPT-5.5) via codex exec; reviewed and verified here. Part of #272 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(policy): add async decision logging with sampling, retention, and query repo DecisionLogger batch-inserts each node's policy decisions straight to the partitioned policy_decisions table via a non-blocking buffered channel (drop-and-count on overflow — logging never adds latency to or fails a decision). Scope decisions sample 1-in-N (default 50, setting policy.decision_log_scope_sample_rate); denials and eval errors always log; input/result JSON samples only at policy.decision_log_verbosity= verbose. Cursor-paginated DecisionRepository backs the upcoming admin log viewer. Retention via partman (daily partitions) and a PolicyDecisionLogCleanupTask honoring policy.decision_log_retention_days (default 14). PDP emits entries per evaluation; the System owns the logger lifecycle and settings hot-reload. Implementation drafted by Codex (GPT-5.5) via codex exec; reviewed and verified here (removed an unused, unsynchronized PDP setter). Part of #272 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(api): add admin policy management API and capability endpoint /api/v1/policy/capability (authenticated feature detection) plus the acting-admin /api/v1/admin/policy surface: vendor Rego viewer, document CRUD with the one-enabled-per-domain conflict mapped to 409, immutable version creation (compile-checked; failed versions persist as audit history with structured row/col errors and can never activate), activate/rollback with synchronous reload + cross-node invalidation via System.NotifyChanged, stateless validate, throwaway-bundle simulate (never touches the live engine, never logs decisions), and cursor-paginated decision-log queries. Routes mount only when the policy system is wired, keeping proxy/transcode modes untouched. Implementation drafted by Codex (GPT-5.5) via codex exec; reviewed and verified here (seeded the FK'd test user; replaced an unchecked fmt.Sscanf with strconv). Part of #272 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(web): add /admin/policy workspace with Rego editor, simulate, and decision log New Policy admin page (System nav group): documents list with one-enabled-per-domain conflict handling, CodeMirror 6 Rego editor (hand-rolled StreamLanguage mode) with server compile issues rendered as inline lint diagnostics, explicit Save-version vs Activate flow with confirm, read-only vendor module viewer, simulate panel with seeded example inputs, version history with rollback, and a cursor-paginated decision-log browser. Capability-gated via /policy/capability. Adds the three decision-log settings to Log Retention. First code-editor dependency in web/ (@uiw/react-codemirror + @codemirror/*), decided in the design spec. Implementation drafted by Codex (GPT-5.5) via codex exec; verified here (lint, format:check, tsc --noEmit, vitest policy suites). Part of #272 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(policy): make OPA authoritative for viewer scope resolution policy.ViewerResolver implements the ViewerResolver interface backed by PDP.ResolveViewerScope and replaces access.Resolver at all five construction sites: router viewer middleware, notifications scopes, the reconciler, jellycompat's scope filter, and the ABS resolver (which now accepts a pre-built resolver, preserving its PIN-at-login semantics). PIN/profile-token verification and disabled-library loading are extracted into shared exported helpers used by both implementations, so the legacy resolver stays compiled as the parity reference with identical behavior. The adapter lives in internal/policy (which already depends on internal/access transitively) — direct typed PDP calls, no new import cycle. Sites without a policy system (proxy modes, bare test routers) keep the legacy resolver until the cleanup phase. Verified: full test suite green (jellycompat TestBeginWebOperation* and one playback GPU test are pre-existing failures, confirmed identical on main), 1368-case parity suite, dedicated ViewerResolver parity/PIN/ nil-vs-empty/fail-closed tests, and a full server boot smoke. Implementation drafted by Codex (GPT-5.5) via codex exec; a first-pass reflection-based adapter was rejected and reworked into the typed in-policy adapter; reviewed line-by-line and verified here. Part of #272 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(policy): make OPA authoritative for acting-admin and permission gates vendor/permission.rego reproduces the acting-admin rule (admin role + primary-profile-or-none), HasEffectivePermission semantics for marker_edit, and the metadata-curation rule including the subtle admin-past-refused-bypass case that requires the explicitly ASSIGNED permission. Policy-backed middleware in policy_gates.go keeps all Go-side lookups (declared-profile primary check, item->library resolution, the 404-on-unknown-item path) and preserves the legacy status/body taxonomy exactly — proven by dual-execution middleware tests that run every scenario through both implementations and assert byte-equal responses. Permission decisions always log (allowed flag populated); simulate and the capability endpoint gain the permission domain automatically via the domain registry. Router swaps behind single constructor choice points with the legacy gates retained for policy-less wiring. Implementation drafted by Codex (GPT-5.5) via codex exec; reviewed and verified here. Part of #272 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(policy): make OPA authoritative for download and playback admission decisions vendor/action.rego decides download eligibility (downloads enabled + user allowed), download-transcode eligibility (transcode enabled + user allowed + artifacts available), and playback admission (stream/transcode counts vs limits, zero = unlimited), with a tightening-only silo_custom.action override that can also clamp a quality ceiling (never widen — merged via quality.min). Go keeps everything stateful: config loading, preset-ladder enumeration, and live session counting. Downloads consult an optional ActionDecider (nil = legacy logic) mapped back to the existing sentinel errors and capability response. Playback gains a minimal AdmissionDecider hook at the exact point of the legacy limit comparison: counts snapshot under the session mutex, PDP evaluated OUTSIDE the lock, then revalidated under lock before insert (retry on count drift) — no admission ever decided on stale counts and no eval under the mutex. Deny reasons map to the legacy ErrTooManyStreams / ErrTooManyTranscodes sentinels, pinned by tests. Parity: combination tables driven against the real PresetsFor / ensureTranscodeAllowed / SessionLimits math; full suite green (known pre-existing jellycompat flakes only). Implementation drafted by Codex (GPT-5.5) via codex exec; reviewed (locking design verified line-by-line) and verified here. Part of #272 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(web): satisfy tsc -b strict return typing in the Rego stream tokenizer The production build (tsc -b) rejects assigning CodeMirror's string | void next() result to string | undefined; tsc --noEmit did not catch it. Restructured the string-literal loop. Part of #272 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(policy): clearer error when a decision is undefined for partial input Vendor policies index required input fields directly, so a hand-written simulate payload missing fields yields an undefined decision. Surface that as 'decision X is undefined for this input (missing required input fields?)' instead of 'empty result' — found while exercising the simulate API against a live server. Part of #272 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(web): set changeOrigin automatically when the API proxy target is remote Remote dev backends sit behind vhost-routing proxies that reject a localhost Host header; local targets keep the existing pass-through behavior. Enables pointing the Vite dev server at a hosted backend via VITE_API_PROXY_TARGET in web/.env.local. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(web): redesign the policy workspace around the decision pipeline The first-pass UI was structurally generic: a five-column document table squeezed beside the editor, three equal-weight action buttons with hidden preconditions, raw version IDs, and jargon copy — nothing taught the model. The page now teaches it: - A pipeline strip states the mental model up front: Silo decides the baseline -> your overrides narrow it -> every decision is logged. Tabs renamed to Overrides / Baseline / Decision Log (ids stay stable for bookmarked URLs). - The document table becomes one card per domain (Library visibility / Admin & permissions / Downloads & playback) with plain-language descriptions, example rules, status pills (Live vN / Draft / Disabled), inline creation, and the enable kill-switch in place. - Selecting an override drills into a full-width editor with a visible lifecycle rail (Draft -> Validated -> Saved -> Live) and one contextual primary action per step; the unedited live source shows no actions until edited. Version comments appear only at the save step. - Simulate is reframed as 'Test before going live' with a human verdict chip (Allowed / Denied — reason / ceiling summary) above the raw JSON; internal generation counters no longer surface. - History uses 'Make live' with plain go-live copy; authors read 'User N'; the baseline tab explains that upgrades never touch overrides. Hand-written redesign (no Codex); verified via vitest, tsc, eslint, prettier, and a production build. Part of #272 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(web): present the policy baseline as readable rules, not raw Rego The Baseline tab dumped five Rego modules into read-only editors. It now leads with what the rules actually do: one card per domain with plain-language statements of the shipped behavior and a note on what an override may change, plus content-rating and playback-quality tier ladders parsed live from the lib module sources (so the tiers shown are the ones the server enforces, not a hardcoded copy). The Rego source stays one click away behind a per-module accordion and remains the stated source of truth; unrecognized modules fall back to source-only. Part of #272 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(policy): add access-groups design addendum Groups with permission toggles become the everyday admin surface; the Rego editor is demoted behind policy.editor_enabled (default off). Restriction-only composition: group grants are an upper bound, per-user settings tighten further — same rule as the existing account/profile merge, one layer up. Part of #272 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(access): add access groups — group defaults with restriction-only composition New access_groups table + users.access_group_id (one group per user, NULL = today's behavior). Group grants are an upper bound composed with the user's own settings by strictest-wins rules — library intersection, MinQuality, AND'd booleans, strictest positive stream/transcode limits, permission-mask intersection, and a requests toggle gating CreateRequest. The merge happens in Go (access.ApplyGroupPolicy / EffectivePolicyForUser) before policy inputs are built, so vendor Rego, the parity suites, and the decision log are untouched; every enforcement surface (viewer scope in both resolvers, permission gates, downloads, playback admission, requests) consumes the effective policy and fails closed on provider errors. Changing a group's quality ceiling bumps its members' access_policy_revision, mirroring the per-user rule. Additive admin API: /admin/access-groups CRUD with member counts; PUT /admin/users/{id} + user DTOs gain access_group_id. Also demotes the Rego editor: policy.editor_enabled (default off, hot-reloaded) drives the capability endpoint's editor_available and 403-gates editor endpoints while the engine and decision logging keep running. Design: docs/superpowers/specs/2026-07-02-access-groups-design.md. Implementation drafted by Codex (GPT-5.5) via codex exec; reviewed (composition core + fail-closed call-site audit) and verified here. DB-backed group-store tests pending local Postgres recovery. Part of #272 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(web): add Access Groups admin page and gate the policy editor New /admin/access-groups: a card grid summarizing each group (member count + key restrictions), drilling into an editor that reuses the same LibraryAccessSelector and quality presets as the user editor, with toggles for downloads/transcoded-downloads/requests, concurrent-stream and transcode limits, and a permissions mask (all-assignable by default, narrowable to specific permissions). Delete warns how many members fall back to the built-in defaults. Copy states the composition rule up front: a group grants the most a member can do; their own restrictions still apply on top. The user editor gains a Group picker and read-only row; the Policy nav entry is now hidden unless the capability reports the editor enabled. Plumbing (types, hooks, user-editor picker, nav gating) drafted by Codex (GPT-5.5); the Groups page hand-built. Verified: 25 tests across the touched suites, tsc, eslint, prettier, and a production build. Part of #272 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(access): seed a Default Group and auto-assign newly created users Adds access_groups.is_default with a partial unique index (one default at most — the profiles is_primary pattern) and seeds a permissive 'Default Group' whose ceiling is a no-op, so assignment never changes anyone's effective access until an admin edits it. The seed is guarded against pre-existing defaults and name collisions; the Down migration only removes the row if it is still untouched. Assignment happens at the single INSERT INTO users choke point (UserRepository.Create): when no explicit group is given, access_group_id is filled by a scalar subquery on the default flag — NULL when no default exists. Every creation path (setup, signup, invites, OAuth, admin create) is covered by construction. Setting a new default via the API atomically clears the previous one in the same transaction. Deleting or unsetting the default is legal: new users then start with no group, which is pre-feature behavior. Implementation drafted by Codex (GPT-5.5); migration guards and the choke-point subquery reviewed line-by-line here. Part of #272 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(web): surface the default access group Cards show a Default badge; the group editor gains a 'Default for new users' toggle (with copy noting existing users are never moved); the delete dialog warns when removing the default that new accounts will start with no group. Part of #272 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(access): ship the Default Group with house-rule ceilings Seed values per product decision: 5 concurrent streams, 5 transcodes, transcoded downloads off, and a permission mask of marker_edit only (metadata curation excluded). Plain downloads and requests stay on. The Down guard matches the new values so it still only removes an untouched seed row. Only newly created users are affected; existing users are never assigned. Part of #272 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(access): retire per-user defaults — the Default Group is the sole default policy Removes both legacy 'user defaults' mechanisms now that the seeded Default Group owns new-user policy: - users.max_streams / max_transcodes column defaults drop from 6/2 to 0 (= unrestricted at the user layer), so group ceilings apply to new signups/invites/OAuth users instead of fighting stale per-user numbers. Existing rows keep their stored values — nobody is silently uncapped on upgrade. - The dead defaults.max_playback_quality / defaults.max_profiles settings validation goes away with its only writer (the User Defaults dialog, removed on the web side). Part of #272 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(web): replace the User Defaults dialog with group-governed creation The Users page's 'User Defaults' dialog (defaults.* server settings) duplicated what access groups now do properly, and its values were only ever form prefill — no backend path applied them. The button now links to Access Groups, and the create-user form seeds unrestricted user-layer values (0 streams/transcodes, any quality, downloads allowed) so the member's group governs; per-user fields remain for tightening individual users. Part of #272 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(access): migrate existing non-admin users into the Default Group Existing users join the seeded Default Group on upgrade so one policy source governs the whole instance. Their per-user limits still holding the retired 6/2 column defaults are normalized to 0 in the same statement so the group's ceilings actually apply; deliberately customized values are preserved. Admin accounts stay ungrouped — scope/action decisions are role-blind, so grouping an admin would cap the server owner on upgrade. Part of #272 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(access): keep admins out of the Default Group and treat group moves as policy changes New-user creation now mirrors the migration's admin exclusion: the default access group is only auto-assigned to non-admin roles, so a fresh server owner no longer inherits the starter group's transcode denial and stream caps. Changing a user's access group now bumps access_policy_revision (the group carries permissions, quality, and limits, exactly like the per-user fields that already bump it) and triggers admin session revocation when the group actually changes. Part of #272 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(policy): enforce marker_edit through the PDP on marker write routes The Rego permission policy owned marker_edit but no Go caller ever consulted it: PUT/DELETE /markers went through a handler-local check that short-circuited admins and read only the user's own permissions, so group permission masks and custom policy overrides were ignored. Marker writes are now gated by router middleware like the other permission surfaces: a PDP-backed RequireMarkerEdit that evaluates the group-merged effective permissions (plus the legacy variant for proxy/test wiring without a policy system). The handler-local check and its user loader are gone. Part of #272 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(downloads): assert device/quality policy facts and honor the quality ceiling The download_transcode action check hard-coded an empty device ID and never asserted the requested quality, and no caller consumed ActionDecision.QualityCeiling — custom download policies keyed on those inputs were silently ineffective. Resolve now threads the request's device ID and requested quality into the action input, and a returned quality ceiling downscales the prepared transcode target (the ceiling applies to what is served, matching the serve-time rule in serveDownloadBytes). FileQuality and the content-rating pair stay intentionally empty for downloads — documented on downloadActionInput: those ceilings are enforced against the served artifact by the scope-derived access filter, and asserting the source's quality would wrongly deny capped transcodes. Part of #272 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(access): align the default-group seed assertions with the migration The DB test still asserted the earlier no-op seed (transcode allowed, unlimited streams/transcodes, null permissions); the shipped migration seeds transcode denied, 5/5 limits, and marker_edit-only permissions, so the test failed on any database with the migration applied. Part of #272 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(policy): lock the Rego sandbox by builtin purity and bound compile work Exclude every nondeterministic builtin from the admin sandbox instead of denylisting names, so OPA upgrades cannot silently expose impure builtins while pure helpers like net.cidr_contains stay usable. Apply the same capabilities to the runtime engine, cap concurrent compile checks, and reject oversized sources before they reach the uncancelable compiler. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(policy): require literal booleans in vendor override and input checks Bare object.get truthiness treated any non-false value as satisfied, so a malformed override 'allowed' value could fail to tighten a base grant and hand-crafted simulate input could flip flag predicates. Compare against literal true so anything else denies. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(policy): surface decision log cleanup failures to the task manager CleanupDecisionLogsOnce now returns the first error alongside the deleted count so a broken partition manager or DB outage marks the scheduled task failed instead of reporting 100% success while policy_decisions grows. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(playback): log admission decider errors before failing closed A policy-evaluation failure was silently mapped to the too-many-streams denial, making an engine outage indistinguishable from a real limit hit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(access): nil-guard the downloads user and restore the ABS legacy resolver effectiveDownloadUser dereferenced policy state before its nil-user check, and the ABS handler lost viewer-scoped filtering entirely when the policy system was unavailable because no legacy access.NewResolver fallback was wired like the other resolver paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(web): address admin policy review feedback - invalidate the version query by version_number, the key usePolicyVersion actually caches under - keep the goPrevious cursor-stack updater pure (Strict Mode double-invoke) - make version history rows keyboard-selectable like the document list - clamp download_transcode_allowed when downloads are disabled so groups cannot save a contradictory record Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(api): cap policy endpoint request bodies at 1 MiB The policy write endpoints (create document/version, set enabled, validate, simulate) decoded JSON bodies without a size limit, so an oversized payload buffered fully in memory before CompileCheck's 256 KiB source cap could reject it. Route all five through a shared decodePolicyRequest helper that wraps the body in http.MaxBytesReader and returns 413 with the repo's standard too_large error shape. Part of #272 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UtnZ2Uewzo959hpneLrtRN * fix(access): forbid deleting or demoting the default access group Deleting the default group (or unsetting its is_default flag) left the server with no default: new non-admin users were then created ungrouped with max_streams/max_transcodes of 0 — unlimited — because the legacy per-user column defaults were retired in favor of the group's ceilings. The store now rejects both operations with ErrDefaultGroupRequired (mapped to 409); promoting another group remains the supported way to move the default, and atomically clears the previous one. The admin UI disables the delete button and the default toggle on the default group and explains the promote-another-group flow. Part of #272 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UtnZ2Uewzo959hpneLrtRN * fix(web): keep unsaved policy drafts when a newer version activates elsewhere The editor state was keyed on the active version's id/sha, so a background refetch after another admin (or another tab) activated a version remounted the editor and silently discarded the dirty draft. PolicyEditorPanel now pins the seed it is editing against and only adopts an incoming seed when nothing can be lost: the editor is clean, the draft already equals the incoming source (the same-admin activate flow), or the selection moved to a different document. Otherwise the pinned editor stays mounted and an inline notice offers an explicit "Load live version" action. Part of #272 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UtnZ2Uewzo959hpneLrtRN * fix(policy): fail reloads on invalid custom sources and surface degraded/apply state A stored custom source that stops compiling used to be silently skipped on reload: the bundle widened to vendor-only for that domain while the generation reported fully applied. Reload is now strict — a bad enabled source fails the reload and the last known-good engine keeps serving. Boot keeps its vendor fallback for availability, but skips are recorded on the engine and exposed (with store-outage reasons) through System.DegradedState and additive degraded fields on GET /policy/capability. Activate/SetEnabled re-run CompileCheck instead of trusting the stored compiled_ok flag. Mutation endpoints also no longer conflate persistence with live apply: activation/enable responses carry additive applied/failed_step/ loaded_generation fields and return 202 when the store change persisted but the local reload failed. Addresses review findings C1, C2, and the degraded-signal gap (6.1). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(policy): type deny reasons across the contract and enforce profile_verified Deny handling used to branch on exact free-text reason strings in three Go consumers, and playback reported ANY unrecognized reason — including custom override free text and engine failures — as a stream-limit error. Decisions now carry a stable reason_code (custom overrides always get custom_denial); downloads, the metadata-curation gate, and playback admission switch on codes, with a new ErrPlaybackNotAllowed -> 403 playback_not_allowed mapping for non-limit denials. Rego tests pin every vendor code. The scope contract's tighten-only profile_verified output was also emitted but never consumed; a policy revocation now surfaces as ErrProfileUnverified (403 profile_unverified) instead of silently proceeding. Addresses review findings 6.2 and C4. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(catalog): close the dual-library disabled-scope bypass in direct item authorization EnsureAccessible, EnsureAccessibleIDs, and FilterAccessibleContentIDs gated library access with allow/deny predicates over a single joined media_item_libraries row, so an item linked to BOTH a passing library and a disabled one satisfied the disabled check via the passing row — a direct-ID bypass of disabled-library scope on the detail, media-file, playback, and download paths. All library access predicates now share one helper (libraryAccessConditions) emitting independent EXISTS / NOT EXISTS subqueries, the semantics GetByIDsWithAccess already used, including the orphan-item membership guard for disabled-only scopes. SQL-shape tests pin every builder and a DB-gated regression test covers the dual-library item end to end. Addresses review finding C3 (plus the same shape in buildFilterAccessibleContentIDsSQL, which the review did not flag). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(downloads): serialize quota check and row creation under a per-user advisory lock The concurrent-download quota was check-then-insert with nothing serializing the pair: parallel creates could all observe free quota before any row existed, bypassing the cap and stacking artifact encode jobs. All four check->insert spans (ephemeral original, artifact-backed, series batch, managed batch) now run inside Repository.WithUserQuotaLock — a pg_advisory_xact_lock keyed by user, so the serialization holds across nodes. The artifact path keeps the limiter-before-Ensure ordering (a rejected request must not leave an encode job behind) by holding the lock across Ensure. Managed-entry replacement stays quota-exempt and lock-free. A DB-gated barrier test races 8 creates against a cap of 1. Addresses review finding C5. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(downloads): assert served quality at create time for original and remux downloads Direct-original and remux downloads serve the source resolution unchanged, but create-time policy checks left file_quality empty — an over-ceiling source registered a row serveDownloadBytes could never satisfy. Resolve now runs a final download action check with FileQuality populated on those two paths (capped transcodes keep the ceiling-on-artifact behavior), a custom override ceiling below the served resolution denies, and quality_ceiling_exceeded maps to ErrQualityUnavailable. The ActionInput contract now documents exactly when file_quality and the rating facts are supplied so custom policy authors are not misled. Addresses review finding C6. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(policy): guard activation against slow overrides and make eval timeouts observable A custom scope override that exceeds the 25ms eval budget compiled fine, activated fine, and then converted to 500s on every authenticated request — server-wide lockout authored in the admin editor. Activation and enable now run GuardEvalCost: the candidate source is evaluated on a throwaway engine against a canned representative input under the live budget, and a source that cannot complete is rejected 422 with ErrPolicySlowEval before it goes live. Runtime timeouts keep failing closed but now carry a distinct ErrPolicyEvalTimeout sentinel, an Error log, and a per-engine counter exposed as eval_timeouts on GET /policy/capability so intermittent near-budget policies are attributable. Addresses review finding C7. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * style: gofmt remediation files Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
430224a1b9 |
perf: cut home-screen, Continue Watching, and Latest latency; cache shared home rails (#292)
* perf(jellycompat,sections): bound resume scan, batch leaf detail progress, widen section concurrency Three low-risk fixes from the section-fetch performance investigation (docs/superpowers/plans/2026-07-03-section-fetch-performance.md): - jellycompat: bound loadProgressPage at resumeScanMaxRows=300 so a single request never pages through more than that many in-progress rows. The cap is unconditional: it also covers the sparse-visible-set case (a heavy watcher whose recent rows are mostly dismissed/superseded, or a Series/Season-only request that matches no leaf in-progress row), where the page never fills and the loop would otherwise scan the entire history — previously an O(history) scan reaching tens of seconds. In the common case the loop exits far earlier, so the cap only bounds the pathological worst case; 300 leaves ample headroom to fill a ~20-item Continue Watching page. Beyond the cap the reported total is a clamped lower bound. Covered by TestLoadProgressPage_BoundsScanForSparseVisibleSet. - jellycompat: batch the leaf-item (movie/episode) progress lookup in GetItemDetailsByIDs via ListProgressWithCompletedHistory instead of a per-item GetProgressWithCompletedHistory (~100 sequential queries for a 50-item detail page). Series keep the per-item episode-rollup path (they own no progress row). Output is unchanged; a batch-lookup failure is now logged rather than silently dropping played state for the whole page. - sections: raise fetchAllMaxConcurrency 4 -> 6 to cut FetchAll wave count for large home layouts, staying within the default 20-conn pool. Part of the home/continue-watching latency work. AI-use disclosure: implemented with AI (Claude) assistance. * perf(jellycompat): keep Latest browse on the cross-library fast path under isPlayed /Items/Latest with isPlayed=false is the highest-frequency compat browse (~10.8k calls/day). The played overlay can't be pushed into SQL, so browse over-fetches and filters locally. The cross-library recently_added fast path (BrowseRecentlyAddedAcrossLibraries: one ~1ms index walk per library) was gated on Offset==0, so a heavy watcher who had already seen the newest items needed a 2nd chunk and fell through to BrowsePage — a whole-catalog MIN(first_seen_at) + GROUP BY HashAggregate over ~147k movies measured at ~755ms per call (0.8-1.6s observed end-to-end). Fetch the entire over-fetch budget (maxScannedRows) in a single merged fast-path walk instead of paging into BrowsePage, so the loop fills from one call. The clamp caveat (MaxLimit=1000 leaves a fall-through only for requestedLimit>200, off the Latest hot path) is documented inline. Part of the home/browse latency work. AI-use disclosure: implemented with AI (Claude) assistance. * fix(jellycompat): scope resume scan cap to resume path and bound the fast-path loop Addresses PR #292 review feedback: - Codex (P2): the resumeScanMaxRows cap was applied unconditionally in the general loop, which also paginates the completed (watched-items) list. Gate it on resumeFiltered so the completed path keeps exact TotalRecordCount and deep StartIndex pagination. Covered by TestLoadProgressPage_CompletedScanNotCapped. - CodeRabbit (Critical): the earlier raw-offset fast-path loop — the default Continue Watching shape and the sections-fallback route — had the same unbounded-scan bug and was not covered by the cap (the existing test forces EnableTotalRecordCount=true, routing around it). Bound it with the same resumeScanMaxRows guard. Covered by TestLoadProgressPage_BoundsFastPathScanForSparseVisibleSet. - CodeRabbit (Minor): tag the doc's fenced example blocks as text to satisfy markdownlint MD040. AI-use disclosure: implemented with AI (Claude) assistance. * perf(sections): cache shared user-agnostic home rails per access scope Home-screen rails that are identical for everyone who can see the same libraries (recently added, recently released, genre, trending on server, most watched, new to library, critically acclaimed, award winners, format showcase, seasonal, mood, trending discover, admin-curated lists, and library collections) were rebuilt from Postgres once per request, per user. Only the overlay on top of each row (watched flags, play position, presigned poster URLs) is actually per-user. Insert a process-global resolved-list cache at the FetchOne choke point in internal/sections. Each cacheable row is built once per access scope, held with a 15m TTL, and refreshed in the background 3m before expiry; singleflight collapses cold-miss stampedes into a single build. The per-user overlay still runs fresh in buildSectionsResponse, so no profile state is ever shared. Random and per-user rows (continue watching, next up, recommendations, hidden gems, forgotten favorites, activity feed, user collections) bypass the cache. The access-scope key captures every access boundary the fetch path enforces -- section identity (type + id + config hash) + item limit + accessible and disabled libraries + max content rating + excluded media types + name prefix + allowed-content-id allowlist -- and nothing per-user, so entries are safely shared. Empty membership is never cached (avoids freezing a transiently empty rail); background refreshes are bounded by a timeout. Scale (analytical, derived from the cache behavior -- not a measured latency): for the user-agnostic rows, Postgres section-query volume collapses from O(rows x concurrent requests) to O(rows x distinct access scopes) per 15m refresh window, because most users share a handful of access scopes. Illustrative -- 40 cacheable rows on a home screen, 1000 concurrent users falling into ~5 distinct access scopes: - before: ~40 x 1000 = ~40,000 section queries per wave of home loads - after: ~40 x 5 = ~200 builds per 15m window (plus one background refresh per row per scope), i.e. a warm home load runs zero section queries for these rows. That is a ~99% reduction in shared section-query load at that concurrency; the win grows with concurrency and shrinks as access-scope diversity rises. Design/plan doc added under docs/superpowers/plans/. * perf(jellycompat): serve per-library Latest via the cached recently-added section A jellyfin-compat per-library /Items/Latest rail is the same user-agnostic list as the native "recently added" library rail -- both order by mil.first_seen_at DESC. It was rebuilt on every request through directContentService.BrowseItems, missing the resolved-list cache entirely. Route per-library Latest for movies and series libraries through the native section fetch instead, so it reuses the shared cache. HandleLatest resolves the library's type once, and for a movies/series library builds a synthetic SectionRecentlyAdded with the same type + config + limit + access scope the native rail uses and calls FetchOne; the per-user overlay (favorites, progress, episode targets, presign) is extracted into buildLatestItemDTOs and shared by both the native and BrowseItems paths, so no overlay logic is duplicated. Cached *models.MediaItem values are read-only -- LocalizeItemModels deep-copies before any presign mutation. To let the two surfaces share one entry, resolvedListCacheKey no longer includes the arbitrary section ID: every cacheable section type derives its membership from type + config + limit + scope, never from its own ID (audited all 14 cacheable types plus the library-collection path; the sole s.ID read lives in the non-cacheable user-collection branch). A native recently-added rail and the compat Latest for the same library + scope now collapse to ONE cache entry, built once and reused. Access-scope isolation is unchanged -- the removed ID never carried access information, and every access boundary (libraries, rating cap, excluded types, content allow-list, name prefix) still keys the entry. Guardrails: the native path is restricted to movies and series libraries; every other library type (ebook, music, manga, mixed) is ignored and keeps its exact BrowseItems behavior -- important because an unfiltered recently-added fetch would otherwise surface non-video items to Jellyfin clients that only expect video. Deeper pages, played-filter and backdrop-required requests, a client asking for a type other than the library's own, and any FetchOne error also fall back to BrowseItems. Chosen over an alternative that gave the synthetic section a deterministic ID (which kept two separate cache entries): both returned identical data with similar complexity, so the shared-entry design won. * fix(sections,jellycompat): post-review fixes for the shared-list cache and Latest path Consolidates fixes from the branch's adversarial review and PR #292 review comments into one commit: - Latest fast path: fall back to BrowseItems when a request carries a genre, name-prefix, or person filter (the synthetic recently-added section cannot express these, so serving it unfiltered would return a wrong, broader set). Eligibility is decided by latestFastPathEligible and covered by a test. - Clamp the /Items/Latest page size to compatBrowseMaxLimit before building the section, matching the BrowseItems fallback, so a large client Limit can't drive an oversized recently-added fetch or explode the shared cache key with unbounded ItemLimit values. - Evict expired entries from the process-global resolvedListCache: resolvedListSet sweeps expired keys at most once per minute, bounding the map to scopes seen within one TTL window. Covered by TestResolvedListCacheEvictsExpiredEntries. - Log a short digest of the cache key (resolvedListLogKey) instead of the raw key in the background-refresh panic/error paths, since the key embeds user-controlled access-scope fields such as NamePrefix. Skipped review comments (verified already fixed or stale against current code): the resume fast-path scan bound and watched-items cap (04d2e795) and the docs fence-language tags (already addressed). Build, vet, and go test -race pass for internal/sections and internal/jellycompat. * perf(plugins): cache plugin installations in-memory, invalidated on lifecycle change ## Problem Every poster/image on a warm home rail re-read plugin_installations from Postgres to answer "is this plugin enabled?" and to acquire the plugin client (Source A: metadata chain buildProviders enabled-check; Source B: ensureClient -> loadInstallation). Plugin-resolved image URLs are never URL-cached, so the plugin source and the DB read behind it fired again on every identical warm request; 100% of images in the target library are plugin-backed. ## Solution - Guarded in-memory installation cache (map[int]*Installation + RWMutex) in plugins.Service. loadInstallation reads through it; the requireEnabled gate stays after the cache read so ErrInstallationDisabled semantics are unchanged. invalidateInstallationCache clears it and is self-registered as a lifecycle hook, so Service.OnLifecycleChange wipes it on install/enable/disable/update/ uninstall. - A generation counter closes an invalidate-vs-repopulate race: captured before installations.GetByID and re-checked under the write lock, so a row fetched before a lifecycle mutation is never written into a freshly cleared cache (would otherwise resurrect a just-disabled plugin). - Route the metadata chain enabled-check through the same cache via a structural InstallationEnabledChecker interface (nil-safe: falls back to the pool query when no checker is injected), wired in cmd/silo/main.go. ## Post-review fix (auto-update reliability blocker) AutoUpdateService mutated installations (new InstallPath, old dir deleted) on the default auto update policy without firing OnLifecycleChange, leaving the cache stale and breaking plugins with "stored plugin manifest mismatch" until restart. It now takes an onChange callback wired to Service.OnLifecycleChange and fires it once per Check run that mutated a row. ## Verification go build/vet, go test ./internal/plugins/... ./internal/metadata/... (-race). Tests: cache hit/invalidation, racing-invalidation guard, IsInstallationEnabled, auto-update fires onChange. ## AI-use disclosure Implemented with AI assistance (Claude). * perf(jellycompat): batch per-item presign, and enrich series on the cached Latest path ## Problem List rails presigned each item's poster/backdrop/logo/still image individually (~160 singular resolver calls for a 40-item page where 4 batched calls suffice), and ItemsHandler carried a near-verbatim duplicate of the batch presigner. ## Solution (batching) Promote the batch presigner to a shared package-level presignCompatListItems (presign_list.go) with a generic collectImagePaths[T]; convert the per-item loops (cached home/Latest rail, favorites, batch loaders, userdata favorites) to one batched PresignImageURLsWithExpiry per image type per page; batch the season/episode collections; delete the three duplicate presign helpers. URL output is unchanged (verified byte-for-byte). ## Post-review fix (series Latest data-parity regression) The native cached Latest fast path built items via compatListItemsFromModels + buildLatestItemDTOs and never ran the series watch-state rollup, so a series library's Latest lost Played / UnplayedItemCount and page 1 disagreed with the BrowseItems fallback. enrichSeriesUserData is promoted to the ContentService interface and called on the native path (reused, not duplicated). ## Verification go build/vet, go test ./internal/jellycompat/... ./internal/catalog/... Tests: bounded presign invocation counts + per-item URL mapping; series rollup populated on the native Latest path. ## AI-use disclosure Implemented with AI assistance (Claude). * perf(sections): gate personalized rails out of the shared cache; widen refresh lead ## Problem 1. The shared home-rail cache whitelisted custom_filter/genre sections by TYPE alone, but those route through fetchFiltered -> ParseQueryDefinition and can carry personalized (per-profile) rules/sorts (watched, favorited, in_watchlist, in_progress, last_watched; sorts progress/date_viewed/plays). Their membership is per-profile yet the cache key excludes userID/profileID, so a personalized rail built for one profile was served to others in the same access scope for up to 15m -- a cross-profile watchlist/watch-state leak. 2. The background-refresh lead was tuned so steady traffic is served a warm entry from a longer soft window. ## Solution - Add QueryDefinition.IsPersonalized() (reusing the existing QueryFieldRequiresProfile/QuerySortRequiresProfile helpers). isCacheableSectionType now parses the section QueryDefinition and refuses to cache custom_filter/genre when personalized; non-personalized definitions stay cacheable. Seasonal/mood/trending build their definitions server-side and stay unconditionally cacheable. - resolvedListRefreshLead 3m -> 10m (soft threshold builtAt+5min instead of builtAt+12min). ## Verification go build/vet, go test ./internal/sections/... ./internal/catalog/... (-race). Test: personalized custom_filter/genre not cacheable; non-personalized are. ## AI-use disclosure Implemented with AI assistance (Claude). * fix(sections,metadata): post-review fixes for shared cache and plugin chain staleness Addresses three review findings on PR #292: - sections: canonicalize section config JSON before hashing so configs differing only in whitespace/field order share a cache entry (native + jellycompat rail sharing). Added TestHashSectionConfigCanonicalizes. - metadata: invalidate the resolved-chain cache on plugin lifecycle changes; the installation-enabled check already reads the invalidated plugin cache, but resolveChainCached could serve a stale provider chain for up to chainCacheTTL after a provider's availability changed. - jellycompat: move ctx to the first parameter of presignCompatListItems for consistency with the other presign helpers. Skipped the episode-image presign batching nitpick: the resolver already dedupes+singleflights, so it is a Minor perf-only item not worth the two-pass refactor risk in this pass. * fix(sections,jellycompat): harden shared rail cache and Latest fast path per review Addresses the eight findings from the deep review of this PR: - Detach the blocking cold-miss rebuild from the singleflight leader's request context (context.WithoutCancel + the shared 30s build timeout) so one client disconnect no longer fails every collapsed waiter and leaves the entry uncached. - Stop client-controlled values minting unbounded cache entries: the compat Latest fast path now always fetches a fixed 100-row budget and slices to the requested limit (one entry per scope+library instead of one per Limit value), and an unrecognized MaxOfficialRating string disqualifies the fast path instead of entering the global cache key. - Add release_date to the sections item projection/scan so movies served via the Latest fast path keep PremiereDate (Jellyfin default-set field) in parity with the BrowseItems fallback. - Fall back to per-item progress lookups when the batched leaf progress query fails, restoring one-item-at-a-time degradation instead of blanking played state for the whole page. - Derive cache eligibility from a single source of truth: fetchSection and isCacheableSectionType now share the userAgnosticSectionFetcher table, whose no-userID/profileID signature makes a fetcher drop out of the cacheable set at compile time if it ever gains per-profile inputs. - Decide Latest fast-path eligibility off the actual browse params the fallback would receive, so any filter later added to buildBrowseParams automatically disqualifies the cached path; share one compatDefaultBrowseLimit constant between both paths. - Extract AccessFilter.WriteAccessScopeCacheKey as the shared, security- critical serializer for all access-scoped caches (resolved-list, editorial candidates, audiobook groups); the editorial key now captures ExcludedMediaTypes, which its loaders already applied in SQL. - Strip leaked agent-transcript markup from the section-fetch plan doc. go build ./..., go vet, gofmt clean; go test -race on internal/sections, internal/catalog, internal/jellycompat passes (TestBeginWebOperation* failures are the known pre-existing flakes). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Quick <31828688+Quick104@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
a5fb16a5c5 |
feat(historyimport): import the Plex account watchlist alongside watch history (#285)
* feat(historyimport): import the Plex account watchlist alongside watch history The Plex import migrated only watch history; the user's saved watchlist had to be rebuilt by hand (#245). - PlexClient gains FetchWatchlist: pages the account-level watchlist on the Plex discover API (discover.provider.plex.tv). It authenticates with the plex.tv ACCOUNT token — the PIN/OAuth session token, which resolvePlexAuth now threads through plexAuth.AccountToken (manual-token imports pass the user token, which doubles as the account token). - Watchlist entries become import Records flagged Watchlisted, carrying identity only (movie/show → KindMovie/KindSeries, guids parsed) and no watch state. They ride the existing matcher (series matching already exists), and matched entries are added to the importing profile's watchlist via the idempotent AddToWatchlistAt — re-imports do not duplicate. A watchlist fetch failure downgrades to a run warning so the history import still completes. - Run summaries gain a watchlist_added counter (new column + repo plumbing + client/admin UI cards). Fixes #245 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(historyimport): count only newly inserted watchlist rows in WatchlistAdded AddToWatchlistAt now reports whether a row was actually inserted (the insert is ON CONFLICT DO NOTHING / INSERT OR IGNORE), and the import summary increments WatchlistAdded only for genuine inserts, so re-importing the same Plex account no longer inflates the count. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Quick <31828688+Quick104@users.noreply.github.com> |
||
|
|
c0f209a936 |
feat(catalog): Latest Episodes sort — order series by newest episode file (#283)
* feat(catalog): Latest Episodes sort — order series by newest episode file Adds a latest_episode_added sort so users can see which shows received new episodes. Today's recently-added surfaces reflect when the SERIES was first added: linking a new episode file never bumps the series' media_item_libraries.first_seen_at (ON CONFLICT DO NOTHING), so a long-running show with a fresh episode sorts as stale (#202). - New denorm media_items.latest_episode_added_at (migration + backfill + partial series index), mirroring the last_air_date_at precedent. Source of truth is episode_libraries.first_seen_at; the three insert paths (UpdateEpisodeLink, BulkLinkEpisodesBySeries, scanner folder restore) bump the parent series atomically in the same statement, monotonically via GREATEST, and only for genuinely new links. - Sort registered in both frameworks: querySortDefs (sections + smart collections + /v1/catalog pick it up automatically via QuerySortFieldSet) and the browse buildOrderByPlan path. - Jellyfin compat: SortBy=DateLastContentAdded now maps to the new sort instead of silently collapsing to series creation date — Jellyfin clients already send this for the TV "Latest" shelf, so they get the correct behavior with no client changes. DatePlayed keeps its old created_at mapping instead of piggybacking. - Web sort picker gains "Latest Episode Added" (series scope). Additive-only per v1 API rules: new sort value, no field/status changes. Part of #202 Fixes #202 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(web): include latest_episode_added in the api QuerySort field union The picker-side QuerySortField gained the value but the api-layer QuerySort union did not, breaking the production tsc build. Part of #202 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(scanner): recompute latest_episode_added_at when episode memberships are removed The denorm was only ever bumped upward (GREATEST) at insert time, but UpdateEpisodeLink also deletes the old episode's library membership on re-link, and reconciliation/path-prefix clears remove memberships too — leaving a stale timestamp that kept the series sorting as recently updated. All removal paths now run in a transaction and finish with a shared full MAX() recompute (catalog.RecomputeSeriesLatestEpisodeAdded) that also resets to NULL when no memberships remain, mirroring the last_air_date_at maintenance pattern. Sequential statements are load-bearing here: data-modifying CTEs are invisible to reads in the same statement, which also silently no-op'd the old path-prefix membership delete. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(jellycompat): keep DateLastContentAdded scoped to series-only requests mapSortBy runs for every /Items browse, so the latest_episode_added mapping leaked into movie and untyped requests where the column is always NULL, destroying the previous created_at ordering. The sort now falls back to created_at unless IncludeItemTypes is exactly Series. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Quick <31828688+Quick104@users.noreply.github.com> |
||
|
|
604bbf1a0f |
feat(playback): unified restart-resilient playback (native + jellycompat) (#174)
* feat(playback): unified restart-resilient playback via shared TranscodeManager Make direct, remux, and native HLS transcode sessions survive a server restart through one shared flow instead of per-method paths. A missing in-memory session becomes a reconstruct trigger, not a 404: the server rebuilds the session from a tiny durable recipe card plus the position the client re-supplies on its next request. - internal/playback/transcode_manager.go: shared TranscodeManager owning the transcodes map, recipe-card lifecycle, reconstruct single-flight + concurrency cap, LoadOrReconstructSession front door, ReconstructSession / ReconstructTranscode, and orphan cleanup. ~90% is logic moved out of the native handler (no behavior change), not new surface. - internal/playback/recipecard.go + recipecard_postgres.go: RecipeCard with a PlayMethod discriminator (direct/remux/transcode; empty decodes as transcode for back-compat) behind a swappable, nil-safe RecipeStore interface backed by transcode_recipes. - internal/playback/session.go: RegisterReconstructed inserts a rebuilt Session under its existing id (no UUID mint, no limit double-count, race-yielding). - internal/playback/transcode.go: CloseProcess keeps the output dir so a reconstruct winner keeps serving; Close removes it. - internal/api/handlers: drain the transcode lifecycle into the manager; wire reconstruct into the stream/segment serve paths; re-bind ownership to the live caller (refuse userID==0/mismatch); card-aware orphan cleanup. - migrations: add transcode_recipes (expires_at TTL, filter-on-read, indexed). Ownership stays two-factor: an authenticated caller AND a session.UserID that matches; the card stores no secrets and identity is re-resolved per request. Tests: recipe-card round-trip/legacy-decode/disabled-noop, RegisterReconstructed insert/race/concurrency, close-vs-close-process dir semantics, the LoadOrReconstructSession status matrix, and the reconstruct concurrency cap. AI-use: implemented with AI assistance (design, implementation, adversarial review). * feat(jellycompat): reconstruct transcodes across restart via shared manager Bring Jellyfin (jellycompat) HLS playback onto the same restart-resilient flow as the native path. Previously jellycompat owned a separate PlaybackHandler with a private transcodes map and a duplicated transcode lifecycle that never grew the reconstruct half, so an in-flight Jellyfin transcode died on restart and the next segment request 404'd. - Embed the shared playback.TranscodeManager and delete the duplicate lifecycle, so jellycompat gets reconstruct, the concurrency cap, the node-affinity rule, and the card lifecycle for free. - internal/jellycompat/playback_sessions_postgres.go: DurableCompatPlaybackStore, a write-through cache over jellycompat_playback_sessions behind the new CompatPlaybackStore interface (nil pool degrades to cache-only). This persists the load-bearing PlaySessionId -> UpstreamSessionID mapping (plus media sources, route item id, seek) so it survives a restart instead of vanishing with the map. - Write a recipe card on compat transcode start keyed by the upstream session id, using the native StreamAppUserID so the ownership re-bind matches; reconstruct the upstream session and the transcode seeked to the requested seg_NNNNN. - migrations: add jellycompat_playback_sessions (expires_at TTL + compat_token index, full PlaybackSession in data JSONB). Auth is mapped to the native user id before reconstruct so the same two-factor ownership check and userID==0/mismatch refusal apply unchanged. Tests: DB-gated (SILO_TEST_DATABASE_URL) durable-store round-trip proving a session written by one instance reloads in a fresh one (the restart case), plus a nil-pool cache-only path; existing handler tests updated to the manager. AI-use: implemented with AI assistance (design, implementation, adversarial review). * docs(playback): consolidate unified playback reconstruction design Replace the three overlapping playback docs (the native Postgres restart-resilience spec, the jellycompat plan, and the unification spec) with a single self-contained design at docs/superpowers/specs/unified-playback-reconstruct.md. The doc leads with the unified design — the one-idea reconstruct model, a strong visual flow of a restart mid-playback, the shared TranscodeManager + recipe card, the two swappable durable stores, security, the concurrency cap and node-affinity constraint, preconditions, and verification. The design history and rationale (reconstruct-not-rehydrate, phased delivery, Redis-vs-Postgres, token-as- descriptor, failure analysis) move to an appendix. It references no other md file. AI-use: written with AI assistance. * fix(playback): address review on restart-resilient playback Four fixes from PR review of the unified reconstruction work: - Rewrite the recipe card on audio-track change. HandleChangeAudioTrack only updated the in-memory session/transcode, so after a restart reconstruct resumed with the stale AudioTrackIndex/TranscodeAudio (and stale play method) from the start-time card. Re-save the card (direct/remux/transcode) with the switched state, mirroring the start-card pattern. - Guard nil TranscodeManager in LoadOrReconstructSession and ReconstructSession. StreamHandler.TM is documented optional (tests/minimal setups); a missing session previously panicked in recipeEnabled instead of returning SessionMissing. ReconstructTranscode already guarded nil; make the two siblings consistent. - Reject direct/remux cards in doReconstructTranscode before spawning ffmpeg, so a non-transcode card id can never enter the HLS reconstruction path. - Log a non-success status from the remote transcode-node DELETE in CloseTranscodeSession; a 401/404/500 was previously silent. AI-use: implemented with AI assistance. * fix(playback): harden restart-resilient compat sessions * feat(playback): token-carried reconstruction across restarts Build on the shared TranscodeManager (introduced earlier in this branch) so a playback session survives an API-server or transcode-node restart without the client re-negotiating, and retire the Postgres transcode_recipes store in favor of a recipe carried inside the signed stream token. - RecipeCard encodes the byte-affecting encode parameters and rides inside the stream token; LoadOrReconstructSession rebuilds the in-memory Session (and, for integrated transcodes, the ffmpeg process) on a cold miss, single-flighted per session and paced by a spawn semaphore. Removes recipecard_postgres.go and the 20260617233705_add_transcode_recipes migration. - transcodenode reconstructs a lost ffmpeg node-side from the forwarded token. - TR-lease: proxy/streamauth enforce a revocation deny-marker on every served segment, with a 500ms Redis timeout, a bounded per-session "allowed" cache (3s TTL, expiry-first graceful eviction), and a degraded-fail-open counter. Review hardening folded in: - Manifest/segment handlers do the in-memory session lookup first and only verify the stream token on a reconstruct miss (token HMAC was per-segment). - Copy-mode reconstruct never applies the encoded-only seg*dur seek, at spawn time or via the recovery path: RestartSeekTarget reports "unresolved" for a copy session whose manifest cannot yet map the segment, so the client retries instead of seeking to a fabricated source time. - Crash teardown is a compare-and-delete (CloseTranscodeSessionIf returns whether it matched); the crash closure tears down the playback session only when it matched, so a session reconstructed under the same id is not killed. - Reconstruct enforces the same per-user stream/transcode caps as a fresh start (RegisterReconstructedWithLimits), closing a token-replay slot bypass. AI-use disclosure: implemented with AI assistance (Claude Code), including a two-round multi-agent adversarial review whose findings drove the hardening. * feat(jellycompat): node-side transcode reconstruct via shared recipe store Make Jellyfin-compat playback sessions survive a server or transcode-node restart by reusing the shared TranscodeManager reconstruct path and a durable recipe store, on top of the durable compat session store added earlier in this branch. - Node-side transcode reconstruct goes through the shared recipe store; the recipe is persisted to the control-plane store (Redis) when a dedicated transcode node is used so the node can rebuild ffmpeg after its own restart. - Adopt the shared manager's API (3-arg OnFFmpegCrash carrying the dead session, guarded CloseTranscodeSessionIf, RegisterReconstructedWithLimits). Review hardening folded in: - Recipe lifecycle: noderecipe.Store gains Delete, called on deliberate teardown (stop, method-switch discard, node stop/force-reload) so a stopped session cannot be resurrected by a buffered request after a node restart; crash paths intentionally keep the recipe so a resume can reconstruct. - Crash closure tears down the upstream session only when the guarded transcode close matched, so a reconstructed successor is never left orphaned. - Copy-mode segment recovery surfaces a retryable not-found instead of a wrong-position restart, matching the native and node paths. - Durable Update is now a SELECT ... FOR UPDATE transaction, removing the lost-update clobber that could silently drop a transcode recipe. - Empty-token route resolution no longer falls back to an unbounded full-table scan; DB expiry filters bind the injected clock; the redundant re-Get is gone. AI-use disclosure: implemented with AI assistance (Claude Code), including a two-round multi-agent adversarial review whose findings drove the hardening. * docs(playback): consolidate restart-resilient playback design Replace the superpowers spec with a single architecture record describing the token-carried recipe card, the shared TranscodeManager reconstruct path for direct/remux/transcode, the jellycompat durable session + node recipe store, and the revocation-lease model with its fail-open tradeoff. AI-use disclosure: written with AI assistance (Claude Code). * docs(playback): correct jellycompat node-recipe rationale in comments The noderecipe / transcode-node / jellycompat comments justified the Redis recipe store with "a Jellyfin client cannot round-trip a token". The real reason: the node-hop token is server-minted and could carry the recipe, but the recipe is mutated in place under a stable session id (a /Sessions/Playing/Progress audio switch restarts ffmpeg without re-minting the client's token) and a third-party Jellyfin client cannot be driven to refresh a stale token, so the node must reconstruct from a server-authoritative, node-reachable store. Aligns the comments with docs/architecture/restart-resilient-playback.md §10. Comment-only; no behavior change. * refactor(playback): remove deny-lease revocation, defer to future PR The deny-lease stream-revocation mechanism (the internal/streamauth package, its silo:streamauth:<sid> Redis markers, the proxy Allowed() enforcement, and the admin Stop/Terminate deny write) only ever enforced on the offload-proxy topology and was a silent no-op on the integrated single box and the dedicated transcode node. Rather than ship a partial revocation feature that looks complete but isn't, remove it wholesale and defer a uniform cross-topology revocation design to a dedicated follow-up. Removed: internal/streamauth (package + tests); the LeaseDenier field, StreamLeaseDenier interface, and denyStreamLease helper in playback.go; the admin deny write; the router/main wiring; and the proxy verifyToken Allowed() gate. The unified-reconstruct core (recipe-token, LoadOrReconstructSession) is orthogonal and untouched. Known limitation (now on every topology): admin Terminate and user Stop tear down the live in-memory session and ffmpeg producer, but a still-valid stream token can reconstruct the session until its 24h TTL expires. No node-side byte-withholding ships in this PR. docs/architecture/restart-resilient-playback.md is updated to mark the revocation/deny-lease sections as deferred and to drop the overstated "instant revocation on admin kill" claim. * fix(playback): allow zero-caller bearer on transcode reconstruct The authless HLS transcode delivery routes (master.m3u8 / segment) treat the session UUID as the bearer credential, so a real request carries requestUserID == 0. The live serve path already allows this, but ReconstructSession hard-rejected a zero caller, so a request that worked before a restart became SessionMissing -> 404 after the in-memory session was gone, breaking the restart resilience these routes advertise. Match the live-path contract in LoadOrReconstructSession: allow a zero caller (UUID-as-bearer) and refuse only a non-zero caller that mismatches the card owner. The reconstructed session is bound to card.UserID either way. Adds TestReconstructSession_Ownership covering both cases. * fix(jellycompat): re-persist recipe on local audio switch A Jellyfin client switching audio on an integrated/local compat transcode restarted live ffmpeg with the new track but did not re-persist PlaybackSession.Recipe. The remote branch already re-persists via startRemoteTranscode -> persistTranscodeRecipe. After a central restart, reconstruct rebuilt ffmpeg from the stale Recipe.AudioTrackIndex, so the integrated session resumed on the original audio track. Persist the updated recipe (best-effort) after a successful Restart in the local branch, mirroring the remote branch, so the durable Recipe.AudioTrackIndex tracks live ffmpeg. Adds a regression test. * fix(playback): strip stream token from proxied transcode-node URL proxyToTranscodeNode appended the client's raw query string to the internal transcode-node URL and logged that URL on transport failure. When a remote transcode runs without a separate proxy node, that query carries ?st=<signed JWT> — a 24h bearer reconstruction descriptor exposing the media path and recipe claims — placing the token into internal requests and error logs. Strip the "st" param before building targetURL, preserving any other query params. The token is neither forwarded to the node nor present in the logged URL. Header-forwarding of the token (so the node can reconstruct) is a separate follow-up (#6). * fix(playback): fail open on transient limit-provider error in reconstruct During the reconstruct wave right after a restart (Postgres under peak load), a transient limit-provider DB error was collapsed into a hard 404, permanently stopping playback for a user within their limits. limitsForUser wrapped any provider error, RegisterReconstructedWithLimits propagated it, and ReconstructSession mapped every error to SessionMissing -> 404 - indistinguishable from a genuine over-cap rejection. Distinguish the two: tag provider errors with a new ErrLimitProviderUnavailable sentinel and, during reconstruct, fail OPEN on a provider error (admit via RegisterReconstructed + log a degraded warning) rather than refuse - mirroring the reliability-first fail-open-on-dependency-error philosophy. A genuine ErrTooManyStreams / ErrTooManyTranscodes over-cap still refuses. Adds tests for both the fail-open and still-refused paths. * fix(playback): forward stream token to transcode node as header The dedicated transcode node's reconstruct path reads the stream token only from the X-Silo-Stream-Token header, but proxyToTranscodeNode forwarded only the node-API bearer token (and #5 now strips st from the URL). So when the central API proxied to the node and the node self-restarted, it could not reconstruct from the recipe-complete native token -> 404. Capture st before stripping it from the URL, verify it at the API boundary (streamtoken.Verify + SessionID match, mirroring the node's own check), and forward it as X-Silo-Stream-Token. Best-effort: a missing/invalid token never blocks the live proxy, and the token is still kept out of the forwarded URL and logs. * fix(playback): restart node ffmpeg on native remote audio switch A native audio-track switch on an offloaded/remote transcode was a no-op at the node yet returned 200 with a fresh URL: HandleChangeAudioTrack restarted ffmpeg only when the API owned a LOCAL TranscodeSession, so for an offloaded transcode the node kept serving the OLD audio (the node consults the token only on a session miss). The replacement URL was also minted from identity- only claims, so a later node restart 404'd. For the offloaded transcode case (detected via session.TranscodeNodeURL), POST a fresh /transcode/start to the node with the new AudioTrackIndex (handleStart tears down and restarts ffmpeg) and mint the replacement proxy URL from a full RecipeCard so reconstruct survives a node restart. The encode recipe is derived from the durable session target fields plus the file, mirroring HandleStartTranscode. A concrete SegmentDuration (playback.DefaultSegmentDuration) is embedded rather than 0: the node's token completeness gate treats SegmentDuration<=0 as incomplete and falls back to a recipe store the native path never populates, which would 404 on a node restart - the exact resilience this path provides. A failed node POST now surfaces 502 rather than a false 200. Remux and non-offloaded (local) transcode paths keep their prior identity-claim URLs unchanged. Known limitation: Session does not persist the original SegmentDuration or SubtitleTrackIndex/SubtitleBurnIn, so a remote audio switch resets subtitle selection to none and assumes the default segment length; a client that started with a non-default segment length will resegment on switch. Making that state durable on the session is a follow-up. * docs(playback): scrub stale deny-lease/revalidator comments The deny-lease revocation mechanism and its "central revalidator" were removed earlier in this branch, but four comments still described them as live (transcode_manager.go, noderecipe/store.go, streamtoken/token.go, proxy/server.go). Reword them to match the shipped behavior: ownership claims are re-resolved at reconstruct, the noderecipe store shares Redis only with the node-session tracker, and a sub-TTL hard cut depends on a node-side revocation mechanism that is deferred to a future PR. * fix(jellycompat): surface durable playback-session write failures DurableCompatPlaybackStore.Update applied the in-memory mutation and then swallowed every Postgres commit-failure path, returning nil. Callers that promise restart resilience (persistTranscodeRecipe's recipe write, the upstream-session binds in streams.go) were told the session was durably persisted when only the cache held it, so a transient DB hiccup could leave the next restart reloading a stale row (wrong audio track) or 404ing. updateDB now returns the genuine DB round-trip error (begin/query/unmarshal/ marshal/exec/commit); Update propagates it while still applying the in-memory mutation so live state stays correct. A nil pool and a genuinely absent/expired row remain best-effort (return nil) — only real infrastructure failures propagate, so existing rollback paths fire exactly when durability is lost. Part of #174 * fix(playback): re-inject stream token into proxied transcode manifests API-proxied remote transcode manifests dropped the reconstruct token from their segment URLs, so playback died after a node or API restart. When a remote transcode has no separate proxy node, the client loads its manifest via the API-local path; proxyToTranscodeNode strips the signed token ("st") from the forwarded URL (keeping it off node URLs and logs, forwarded only as the X-Silo-Stream-Token header), and the node builds relative segment URIs from that token-less query. The segment URLs the client received carried no token, and the proxy only re-attached the header when an incoming segment request already had "st" — which it never did — so a restart made those segments non-reconstructable and they 404'd. proxyToTranscodeNode now rewrites the manifest body at the boundary: every segment and #EXT-X-MAP init URI gets the client-facing, API-verified token re-appended (new playback.AppendManifestQueryParam helper), so the client's later segment fetches carry "st" again and reconstruct after a restart. The token still never reaches the node URL or its logs. Only 200 .m3u8 responses are rewritten (Content-Length corrected); segments stream through untouched. Part of #174 * fix(playback): preserve subtitle/cadence recipe across offloaded audio switch Switching audio on a remote (offloaded) transcode with burned-in subtitles silently dropped them, and reset a non-default segment cadence. The offloaded audio-switch restart rebuilt the node start request from Session state, but Session/SessionStreamState retained no subtitle or segment-duration state (only the live local ts.Opts() and the RecipeCard did), so the branch hard-coded SubtitleTrackIndex:-1, SubtitleBurnIn:false and SegmentDuration:Default — signing that altered recipe into the replacement stream token. An audio switch then changed bytes beyond audio selection, and any later reconstruct kept the wrong no-subtitle/wrong-cadence recipe. Persist the byte-affecting recipe on the session: SubtitleTrackIndex, SubtitleBurnIn and SegmentDuration are added to Session/SessionStreamState, populated at start (finalizeTranscodeStart) and on post-restart reconstruct (ReconstructSession from the card), carried forward on every audio-switch state update, and read back when rebuilding the offloaded node request and its recipe card. The restart now reproduces the exact live stream. Also resolves the M-4b non-default segment_duration reset. Part of #174 * fix(playback): serialize transcode spawn paths with a per-session lock Reconstruct was single-flighted only against other reconstructs, so a restart-driven segment reconstruct racing a quality/seek/audio fresh start could spawn two ffmpeg processes writing the same output directory at once — segment corruption, partial-write closes, orphaned processes, and skewed active-job accounting. The atomic register-after-spawn (GetOrRegister / the reconstruct compare-on-register) prevented a map leak but not the concurrent disk writers, because the losing path had already spawned. The dedicated transcode node had the same split between handleStart and spawnReconstruct. Add a refcounted per-session lifecycle lock to both TranscodeManager and the node Server, held across "check existing -> spawn -> register": - reconstruct (doReconstructTranscode / spawnReconstruct) re-checks under the lock and yields to any live session instead of spawning a duplicate; - the native and jellycompat fresh-start paths take the lock around their spawn+register (the native path also closes any session a reconstruct rebuilt in the meantime so its fresh ffmpeg is the sole writer); - the node handleStart holds it across teardown+spawn+register. The refcount drops the map entry once no path holds/waits, keeping it bounded. GetOrRegisterTranscodeSession is removed — the lock supersedes it and keeping a register-after-spawn primitive would invite reintroducing the race. Part of #174 * fix(playback): serialize restart re-spawn under the session lifecycle lock TranscodeSession.Restart() releases s.mu across cancel -> wait-for-done -> re-exec and spawns ffmpeg into opts.OutputDir without holding the per-session lifecycle lock. LockSessionLifecycle's contract (fresh start, restart, reconstruct) requires restart to hold it too, but all five callers invoked Restart unlocked: native audio-switch and segment-recovery, compat audio-switch and segment-recovery, and the transcode-node segment-recovery. A restart racing another restart (audio-switch vs segment-recovery) or a fresh-start/reconstruct could land two ffmpeg processes writing the same segment directory -- mixed timelines, init.mp4/segment mismatch, and an orphaned-but-still-writing ffmpeg -- the exact concurrent-writer corruption the lifecycle lock exists to prevent. Add RestartSessionLocked (TranscodeManager) and restartSessionLocked (node Server) that hold LockSessionLifecycle only across the cancel->respawn transition, re-check that the handle is still the live mapped session under the lock, and return ErrSessionSuperseded rather than re-spawning a stale handle. Route all five call sites through them. The lock is released before callers wait on segments so recovery latency is unchanged. Tests: gating (restart blocks until the lifecycle lock frees, then spawns), concurrent-restart serialization, and superseded re-check on both the manager (covers native + compat) and node lock owners. --------- Co-authored-by: Quick <31828688+Quick104@users.noreply.github.com> |
||
|
|
287ff3658f |
fix(jellycompat): keep still-playing direct-play sessions in the activity view (#262)
* test(jellycompat): implement ListProgressSince on userstore test fake PR #258 added ListProgressSince to the userstore.UserStore interface but missed the progressCountingStore stub, leaving the jellycompat test build broken on main. Stub it with the same panic("unused") the sibling methods use. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(jellycompat): keep still-playing direct-play sessions in the activity view An Infuse Static=true direct play skips PlaybackInfo, so its playback reports carry a client-generated PlaySessionId. handlePlaybackReport only resolved reports by exact PlaySessionId, so every report silently no-op'd: the admin Activity View position froze, nothing refreshed the upstream session's activity, and stale cleanup reaped the session ~45s in while the client kept playing. Once reaped, ensureUpstreamPlayback early-returned the dangling session id forever, so the stream card never came back. Fix all three legs: - handlePlaybackReport falls back to the same token-scoped route lookup the stream path uses (ItemId, then MediaSourceId) when the PlaySessionId is unknown, and revives a reaped upstream session when a progress report proves the client is still playing. - HandleVideoStream marks an in-flight media transport around direct-play and remux serving, mirroring the native stream handler, so long-lived range transfers keep the session alive without progress reports. - ensureUpstreamPlayback verifies the upstream session still exists before reusing it, recreating it under the same play session when it was reaped. FindByRoute now compares RouteItemID with UUID normalization so report ids match regardless of client dash/case formatting. Closes #244 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(jellycompat): harden report fallback and upstream revive per adversarial review Address three review findings on the issue #244 fix: - Deterministic report binding: the stream path now records the client's own PlaySessionId (Static=true direct play never learns the server id) as an alias on the play session it binds, and playback reports resolve by that alias before falling back to the ambiguous item/source route scan. A Stopped report that only matched by bare route no longer tears the session down (it may not own it when the same item plays twice under one token); stale cleanup owns that path. - Same-method revive closes any transcode still keyed to the reaped upstream id before recreating the session, so a second ffmpeg cannot start alongside an orphaned one. - Attaching a new upstream session is now a compare-and-swap on the observed upstream id: the loser of a concurrent revive/range-request race stops its session and adopts the winner instead of leaving an orphan counted against the user's stream limits. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(jellycompat): guard alias ambiguity and method switches in upstream attach Second-round adversarial findings on the issue #244 fix: - A client PlaySessionId alias now only resolves when it identifies exactly one live session for the token, and the candidate must agree with the report's ItemId/MediaSourceId. A reused or stale client id degrades to the route scan (progress still lands on the right item) instead of binding an arbitrary session, and an ambiguous Stopped report tears nothing down. - A CAS loser only adopts the concurrent winner when it serves the same play method; a concurrent method switch surfaces 409 Conflict instead of continuing with mismatched transcode bookkeeping. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(jellycompat): key audio-selection updates by the resolved play session id Review follow-up (PR #262): after an alias or route fallback, req.PlaySessionID is the client's own generated id and not a playback store key, so the audio-track-change branch in handlePlaybackReport silently dropped selection updates for Static direct-play sessions. Use the resolved playSession.ID for the store mutation and logging. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
d1fd7c236f |
fix(jellycompat): sync activity dashboard immediately on compat playback start/stop (#261)
* fix(jellycompat): sync activity dashboard immediately on compat playback start/stop Compat (Infuse/Jellyfin) sessions were only reconciled into playback_sessions_sync on the periodic 15s reconciler tick, so stopped streams lingered in the admin activity dashboard and overlapped with newly started ones as ghost sessions. Native playback handlers already trigger an immediate SessionSyncer.SyncNow on start/stop; wire the same syncer into the jellycompat playback handler and flush after teardownPlaySession (Stopped report + ActiveEncodings teardown) and after a new upstream session starts. The stop-path sync detaches from request cancellation (context.WithoutCancel) because clients often drop the connection right after reporting a stop. Also adds the ListProgressSince method to the jellycompat test fake so the package's tests compile again after the downloads-v2 interface change. Part of #205 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(worker): serialize SyncNow and bound compat request-path session sync Adversarial review follow-ups: SyncNow snapshots could commit out of order when request-path syncs raced the periodic tick (an older snapshot committing last would resurrect stopped sessions or drop fresh ones), and the detached stop-path sync had no deadline, letting a stalled DB pin request goroutines. Serialize snapshot capture + reconcile under one lock and cap request-path syncs at 5s so failures degrade to the periodic tick. Part of #205 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(worker): coalesce immediate session syncs instead of queueing on a lock Review iteration 2: a stop-triggered sync queued on a bare mutex could expire its 5s deadline waiting behind a slow tick and fail before removing the stopped row. Coalesce instead: one owner reconciles at a time, callers that arrive mid-flight return immediately after flagging a follow-up pass, and the owner re-captures a fresh (post-change) snapshot afterwards. Request goroutines never block behind another sync, snapshots still commit in capture order, and an expired-context owner leaves the queued pass for the next tick instead of burning it. Part of #205 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
|
eafa22a3c3 |
fix(jellycompat): improve audio track selection and display titles (#240)
- preferredAudioStreamIndex: when default track is TrueHD/MLP and device profile doesn't explicitly list that codec, falls back to best compat track (EAC3 > AC3 > DTS > AAC…), same language preferred. Prevents Android TV clients receiving undecoded lossless passthrough streams. - audioTrackDisplayTitle: structured display title (e.g. "English - EAC3 5.1") replacing the raw firstNonEmpty fallback. Suppresses generic embedded labels (Stereo, Surround) that add no information. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> |
||
|
CoffeeKnyteandGitHub
|
652b5d1e9f |
perf(jellycompat): fast-paths for slow Jellyfin-compat queries (#247)
* perf(jellycompat): fast-paths for slow Jellyfin-compat queries
Rebuilds the jellycompat slow-query fast-paths cleanly on main, without
the hub-sections virtual-library feature (which is intentionally absent
on this branch). Each path is bounded so behavior stays predictable
under load.
Fast-paths:
- Episode catalog: add the load-bearing `si.type = 'series'` hydration
guard plus a matching entry-scan series-parent guard, so the page and
count scans stay aligned with hydration (no under-filled pages, no
inflated TotalRecordCount from podcast/audiobook episode rows).
- /Shows/{id}/Episodes AdjacentTo: serve a bounded prev/self/next
neighbor window with series-membership validation and a cross-series
id guard, falling back to the full listing.
- Resume: serve Continue Watching through the capped native
continue-watching fetcher (next-up suppressed, in-progress only)
instead of loadProgressPage's unbounded scan. Wires only the
sections read-time fetcher (StoreProvider) into ItemsHandler; no
hub-section/virtual-library exposure.
- Played-history: push the type + library filter down into SQL.
- recently_added: fan a no-parentId browse out across libraries with an
app-side dedup merge, clamped to BrowseFilters.MaxLimit, and exclude
compat-hidden (audiobook/podcast) libraries from the fan-out.
- Detail fields: batch per-item GetItemDetail in the Latest/browse
detail-field paths via GetItemDetailsByIDs, with per-item fallback.
- Search: term-length and result-count guards for aux search.
* perf(jellycompat): clamp Resume fast-path fetch to maxResumeItems
A large client-supplied StartIndex on /UserItems/Resume inflated
loadResumeViaSections' fetchLimit (pageSize + StartIndex) and fed it
straight into ResolvedSection.ItemLimit, letting the continue-watching
fetcher scan up to its continueProgressMaxScanned hard cap (1000 rows)
instead of stopping near maxResumeItems. Clamp fetchLimit to
maxResumeItems and short-circuit to an empty page when StartIndex is at
or beyond the cap (no visible row can land there), keeping the capped
fetcher's scan bounded and predictable under load.
Addresses CodeRabbit review finding on PR #247.
|
||
|
CoffeeKnyteandGitHub
|
1dbcf2cd9a |
fix(jellycompat): guard aux search paths and index-back person search (#252)
* fix(jellycompat): guard aux search paths and index-back person search
Short, recursive type-ahead terms (e.g. a single "a") against the
PostgreSQL people index and the in-memory collection/box-set filter
produced multi-second scans that pegged the server when a client fired
one search per keystroke. Every jellycompat search path except the
Meilisearch-backed /Items media search now rejects a provided SearchTerm
shorter than 3 runes without touching any backend, and caps results at
20 regardless of the client-requested Limit:
- /Persons (PostgreSQL people scan)
- /Search/Hints (catalog search)
- /Items BoxSet (in-memory collection filter)
Shared policy + helpers live in search_guard.go (auxSearchMinTermLen=2,
i.e. reject 1-2 runes / allow 3+, and auxSearchMaxResults=20) with unit
coverage. The 3-rune floor is deliberate: 3 runes is the point where a
pg_trgm trigram index becomes usable, so the gate lines up with the index
and still lets legitimate short titles/names ("300", "Saw", 3-letter
actors) and 3-char type-ahead hints through.
PersonRepository.Search filters with `name ILIKE '%'||$1||'%'` rather
than `LOWER(name) LIKE '%'||LOWER($1)||'%'`. The old expression filtered
on LOWER(name), which the trigram GIN index idx_people_name_trgm (built
on name) could not serve, so every search fell back to an ordered index
scan on idx_people_name that walked the whole table for rare terms
(~300-400ms on the 889k-row production people table). ILIKE on name lets
pg_trgm serve rare 3+ char terms from the trigram index, while the planner
still picks the ordered btree scan with early termination for common
terms. ILIKE is case-insensitive, so behavior is preserved (including the
pre-existing treatment of % and _ in the term as LIKE wildcards).
Verified on production silo-postgres (889,302 people), parameterized query
under both custom and generic plan cache modes:
rare 3-char 'qzx': 304ms -> 1-8ms (trgm bitmap index)
rare 4-char 'zzzz': 329ms -> 1-2ms (trgm bitmap index)
common 3-char 'ann': 15ms -> 7-74ms (btree early-stop / trgm bitmap)
New worst case across all terms is ~83ms (generic-plan common 3-char).
* fix(jellycompat): stop gating meilisearch hints, clamp box-set search
Two review follow-ups on the aux-search guards:
- HandleSearchHints is served by the catalog (Meilisearch-backed) search
provider, which already bounds and short-term-handles its own results.
Gating it with auxSearchTermTooShort contradicted the guard's own
"non-Meilisearch paths only" policy and hid valid 1-2 char titles
("Up", "It") from global type-ahead. Drop the too-short gate there;
keep the empty-term check and the result clamp.
- handleBoxSetsList gated short terms but passed query.limit straight to
slicePage, which treats <=0 as no cap, so a box-set *search* was
uncapped unlike Persons/Hints. Clamp the limit when a search term is
present; empty-term browse keeps its client paging window.
|
||
|
QuickandGitHub
|
8afe783277 |
fix(jellycompat): honor StartItemId episode queues (#242)
* fix(jellycompat): honor StartItemId episode queues * fix(jellycompat): compact episode queue models |
||
|
QuickandGitHub
|
a0fbe2cdda |
[codex] feat(jellycompat): use catalog search provider (#236)
* feat(jellycompat): use catalog search provider * fix(jellycompat): route video search buckets to provider |
||
|
|
02e62767a1 |
feat(watchsync): sync watchlists with Trakt/Simkl/MDBList (#227)
* feat(watchsync): sync watchlists with Trakt/Simkl/MDBList Extend the watch-providers feature to sync a user's watchlist, generalizing the existing favorites pipeline rather than duplicating it. What changed - Generalize the favorites sync into one ListKind-parameterized pipeline (internal/watchsync/lists.go) driving both favorites and watchlist; the per-favorites service methods are replaced by kind-generic ones. The shadow table watch_provider_favorite_items becomes watch_provider_list_items with a list_kind discriminator. - Providers: Trakt gains watchlist sync (/sync/watchlist, distinct from favorites); Simkl gains plan-to-watch sync; MDBList is re-mapped from favorites to watchlist (its only list is a watchlist) — its capabilities now report import_favorites=false / import_watchlist=true, and the migration re-binds existing MDBList connections. - Auto-remove watched items from the watchlist: a standalone, default-on profile preference (user_profiles.remove_watched_from_watchlist) removes a movie when watched and a series once every episode is watched. Implemented as watchstate.CompletionObserver (internal/watchlist.Maintainer), wired into the manual mark-watched, playback-stop, and jellycompat mark-played paths. - Optional MDBList sort-order mirroring: an opt-in, capability-gated toggle mirrors MDBList's watchlist order into Silo via user_watchlist.sort_index; ListWatchlist orders by sort_index then added_at, so both /api/v1/watchlist and the catalog watchlist view inherit it. - Real-time + scheduled: local add/remove pushes to connected providers immediately (removals gated by the opt-in removals toggle); the hourly job is the inbound/import + retry/reconcile path. - Web: watch-provider settings gain watchlist import/export/removals and "mirror watchlist order" toggles plus watchlist sync stats. Why - The favorites and watchlist pipelines are ~90% identical; generalizing keeps one code path (per CLAUDE.md's anti-duplication guidance) instead of cloning. API/compat - All new fields on ConnectionStatus/Capabilities/ConnectionUpdate/SyncRun and the web types are additive (Silo v1 additive-only rule). No existing field is renamed, removed, or retyped. Risks / follow-up - MDBList capability flip is intentional and client-visible: silo-android / silo-apple may need to surface MDBList under the watchlist (not favorites) UI. - MDBList existing users: their MDBList list previously mirrored Silo favorites and now mirrors Silo watchlist; the first post-migration sync is a union (removals default off), so nothing is destructively purged. - Order mirroring reflects the order MDBList returns from /watchlist/items (couldn't confirm against their docs — Cloudflare-blocked); if it ever diverges from the UI sort, a sort param is the small follow-up. Tests: new maintainer (auto-remove) and watchlist-order unit tests; provider + service tests updated. go build, go test (affected pkgs), migrate-validate, verify-local-paths, web prettier/eslint/tsc all pass. AI-use disclosure: implemented with Claude Code (Claude Opus 4.8). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(watchsync): update list shadow table references * fix(watchsync): address review — retry/progress + error propagation Addresses CodeRabbit review on #227: - maintainer: propagate transient catalog lookup errors instead of silently treating every items.GetByID failure as "maybe an episode". - exportList: mark every queued item not confirmed sent (not_found, failed, or omitted) so the pending loop always advances; the next run's upsert clears the error and re-attempts, so transient failures still retry. - removePendingListItems + realtime removal: treat Sent and NotFound as reconciled; leave true failures pending (no last_error, which would strand them from the removal query) so the scheduled run retries, using in-memory dedupe to terminate the loop. - exportLocalListItems: send the normalized items (with computed ProviderItemKey), not the original event slice. - UpdateConnection: clear mirrored watchlist order before persisting the disable and propagate failures, so a failed clear can't report "disabled" while sort_index ordering is still active. - web: include favorite + watchlist removal counts in the exported "sent" total. - test: align serviceFakeRepo list-state with Postgres (clear last_error on successful transitions); add maintainer error-propagation test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
|
a3aa93534d |
fix(jellycompat): authenticate Android TV token-less direct play (#200)
* fix(jellycompat): authenticate Android TV token-less direct play
Stock Jellyfin Android TV ignores the api_key-bearing DirectStreamUrl
returned from PlaybackInfo and builds its own direct-play URL with no
auth header, no api_key/ApiKey query param, and no PlaySessionId. The
media request arrives via the player's HTTP stack (okhttp) with
auth_kind=none, so PlaybackSessionAuth 401s it — the client retries,
falls back to a transcode that stalls, and surfaces "player error".
Add a third fallback in PlaybackSessionAuth, scoped strictly to the
direct-play video stream routes (/Videos/{id}/stream[.{container}]) via
the chi route pattern so /Items/{id}/Download stays protected: anchor
auth on the PlaybackSession negotiated during the preceding (already
authenticated) PlaybackInfo, looked up by mediaSourceId when present
(else the route item id), and resolve its CompatToken.
Covered by tests: token-less direct play with a matching session
succeeds, no matching session 401s, and Download is not loosened.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(jellycompat): require session item match + expand direct-play tests
Address CodeRabbit review on PR #200:
- Require the matched PlaybackSession's RouteItemID to equal the requested
route item before authorizing, so a mediaSourceId cannot authorize a
stream for a different item.
- Seed the compat session in the 401 tests so they fail on route/session
scoping rather than a missing session.
- Table-drive the positive test across /Videos/{id}/stream and
/Videos/{id}/stream.{container}, plus the route-item lookup branch when
mediaSourceId is absent; add a cross-item denial test.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
185a90b048 |
fix(jellycompat): honor client subtitle selection in PlaybackInfo (#222)
* fix(jellycompat): honor client subtitle selection in PlaybackInfo Jellyfin clients send the chosen subtitle as SubtitleStreamIndex on PlaybackInfo, but the compat layer only ever advertised the media's embedded default subtitle (or none). Picking a subtitle before playback had no effect and the video started without it — and an explicit "subtitles off" was ignored when the media had a default subtitle. Mirror the existing audio-selection plumbing for subtitles: - parse SubtitleStreamIndex from the PlaybackInfo body and query - persist it as SelectedSubtitleStreamIndex on the play-session source - advertise it via DefaultSubtitleStreamIndex and flip IsDefault onto the chosen stream (embedded, external, or Silo-downloaded) - honor a negative index as an explicit "subtitles off" The selection is validated against streamable tracks (bitmap subs that require burn-in are excluded, matching delivery) and the downloaded subtitle range, falling back to the media default when invalid. Out of scope (follow-ups): bitmap-subtitle burn-in transcode wiring and mid-playback subtitle changes on progress reports. Part of #217 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(jellycompat): handle downloaded-subtitle lookup failure in selection Don't let a transient ListDownloadedSubtitles error masquerade as "no downloaded subtitles", which silently downgraded a valid subtitle selection to the media default. The lookup error is now logged, and resolution honors a requested index it cannot validate (it may be a downloaded subtitle) instead of discarding the user's choice, while embedded/external selections continue to resolve normally. Addresses CodeRabbit review feedback on #222. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |