Commit Graph
7 Commits
Author SHA1 Message Date
Quick 181bb93333 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.
2026-07-27 00:14:07 +00:00
Quick 61448d6161 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.
2026-07-27 00:12:05 +00:00
QuickandClaude Opus 5 a1bf79c5f7 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>
2026-07-26 23:22:48 +00:00
QuickandClaude Opus 5 790250d87f 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>
2026-07-26 15:24:33 +00:00
QuickandClaude Opus 5 d8faf83c6e 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>
2026-07-26 15:24:33 +00:00
QuickandClaude Opus 5 f4f7cbd8d6 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>
2026-07-26 15:24:33 +00:00
QuickandClaude Opus 5 5933ac0bf2 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>
2026-07-26 15:24:32 +00:00