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.

cmd/silo/main.go

@@ -401,7 +401,10 @@ func main() {
 	// not a runtime condition, so failing here — loudly, before the first
 	// request — is the whole point: the alternative is shipping an image whose
 	// contract disagrees with the clients that vendored it.
-	contract := settingscontract.MustLoad()
+	contract, err := settingscontract.Load()
+	if err != nil {
+		log.Fatalf("settings contract: %v", err)
+	}
 	contractETag, err := settingscontract.ETag()
 	if err != nil {
 		log.Fatalf("settings contract: %v", err)

internal/api/handlers/settings_contract_test.go

@@ -78,17 +78,28 @@ func TestRegistryDefaultsMatchTheContract(t *testing.T) {
 		}
 
 		t.Run(registryKey, func(t *testing.T) {
-			contractDefault, err := scalarDefault(def.DefaultValue)
-			if err != nil {
-				t.Skipf("contract default is not a scalar: %s", def.DefaultValue)
-			}
+			// The null case is settled before scalarDefault, which rejects null
+			// as non-scalar. Asking it first skipped the subtest and left the
+			// comparison below unreachable, so a nullable contract default could
+			// drift from the registry without failing anything.
+			//
 			// The legacy registry stores every value as a string and has no way
 			// to say "unset", so it spells that as the empty string. The
 			// contract spells it null, which is why the language settings are
-			// nullable. Those are the same statement, not a disagreement.
-			if spec.DefaultValue == "" && string(def.DefaultValue) == "null" {
+			// nullable. Those are the same statement, not a disagreement — but
+			// null against a non-empty registry default is a real one.
+			if strings.TrimSpace(string(def.DefaultValue)) == "null" {
+				if spec.DefaultValue != "" {
+					t.Errorf("default disagrees: settingsRegistry has %q, contract has null",
+						spec.DefaultValue)
+				}
 				return
 			}
+
+			contractDefault, err := scalarDefault(def.DefaultValue)
+			if err != nil {
+				t.Skipf("contract default is not a scalar: %s", def.DefaultValue)
+			}
 			if spec.DefaultValue != contractDefault {
 				t.Errorf("default disagrees: settingsRegistry has %q, contract has %q",
 					spec.DefaultValue, contractDefault)

internal/audiobooks/abs_smart_collection_store.go

@@ -22,8 +22,10 @@ import (
 // collection_type = 'smart'.
 //
 // abs.SmartCollection.IsPublic maps to user_personal_collections.is_shared.
-// profile_id is a text column (NOT NULL DEFAULT ”) in the canonical
-// schema, so the empty string stands in for "primary profile".
+// profile_id is a text column in the canonical schema, NOT NULL and
+// defaulting to the empty string, which stands in for "primary profile".
+// (Spelling that default as a pair of SQL quotes here would not survive
+// gofmt, which folds them into a typographic quote in a doc comment.)
 //
 // abs.SmartCollection.Color and abs.SmartCollection.IsPinned have no
 // canonical columns (deferred per spec §6); reads always return the zero

internal/settingscontract/load.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io/fs"
 	"path"
+	"strings"
 	"sync"
 
 	"github.com/santhosh-tekuri/jsonschema/v6"
@@ -188,6 +189,12 @@ func compileObjectSchemas(fsys fs.FS) (map[string][]byte, map[string]*jsonschema
 			continue
 		}
 		name := entry.Name()
+		// schema_ref can only name a .json file, so anything else here is not a
+		// value schema. Parsing it anyway would turn a stray file — an editor
+		// backup, a .DS_Store — into a panic at startup via MustLoad.
+		if !strings.HasSuffix(name, ".json") {
+			continue
+		}
 		body, err := fs.ReadFile(fsys, path.Join(schemasDir, name))
 		if err != nil {
 			return nil, nil, fmt.Errorf("reading value schema %s: %w", name, err)

web/src/hooks/themePreferences.ts

@@ -1,4 +1,5 @@
 import { appearanceCache, storage } from "@/utils/storage";
+import { useOptionalAuth } from "@/hooks/useAuth";
 import type { ThemeId } from "@/lib/themes";
 import { DEFAULT_THEME, THEME_IDS } from "@/lib/themes";
 
@@ -26,6 +27,23 @@ export function appearanceCacheOwner({ loading, user }: AppearanceAuth): string
   return !loading && user ? String(user.id) : null;
 }
 
+/**
+ * `appearanceCacheOwner` for the currently authenticated session.
+ *
+ * The three appearance providers each need the same owner token, and each was
+ * adapting the auth context to `AppearanceAuth` itself with identical code.
+ * That put the shape of auth back in three places, which is the duplication the
+ * doc comment above argues against: widening ownership has to be a change to
+ * this file alone.
+ */
+export function useAppearanceCacheOwner(): string | null {
+  const auth = useOptionalAuth();
+  return appearanceCacheOwner({
+    loading: auth?.loading ?? false,
+    user: auth?.user ? { id: auth.user.id } : null,
+  });
+}
+
 export function isValidTheme(value: string | null | undefined): value is ThemeId {
   return typeof value === "string" && (THEME_IDS as readonly string[]).includes(value);
 }

web/src/hooks/useCustomTheme.ts

@@ -1,7 +1,6 @@
 import { useCallback, useEffect, useRef, useState } from "react";
 import { useSettings, useSetSetting } from "@/hooks/queries/settings";
-import { useOptionalAuth } from "@/hooks/useAuth";
-import { appearanceCacheOwner } from "@/hooks/themePreferences";
+import { useAppearanceCacheOwner } from "@/hooks/themePreferences";
 import { appearanceCache, storage } from "@/utils/storage";
 import { parseVarsJson } from "@/lib/themeExport";
 import { sanitizeCss } from "@/lib/cssSanitizer";
@@ -29,13 +28,9 @@ interface UseCustomThemeResult {
 }
 
 export function useCustomTheme(): UseCustomThemeResult {
-  const auth = useOptionalAuth();
   // Owner of the localStorage warm start; null while auth bootstraps or when
   // nobody is signed in, which keeps the last look on the login screen.
-  const cacheOwner = appearanceCacheOwner({
-    loading: auth?.loading ?? false,
-    user: auth?.user ? { id: auth.user.id } : null,
-  });
+  const cacheOwner = useAppearanceCacheOwner();
   const loadApi = cacheOwner !== null;
 
   // API values

web/src/hooks/useDateTimeFormat.tsx

@@ -14,8 +14,7 @@ import type {
   TimeFormatPreference,
 } from "@/lib/datetime";
 import { useSettings, useSetSetting } from "@/hooks/queries/settings";
-import { useOptionalAuth } from "@/hooks/useAuth";
-import { appearanceCacheOwner } from "@/hooks/themePreferences";
+import { useAppearanceCacheOwner } from "@/hooks/themePreferences";
 import { appearanceCache, storage } from "@/utils/storage";
 
 export const DATE_FORMAT_SETTING_KEY = "ui.date_format";
@@ -49,13 +48,9 @@ const DateTimeFormatContext = createContext<DateTimeFormatContextValue | null>(n
  * lib/datetime, and exposes setters for the settings UI.
  */
 export function DateTimeFormatProvider({ children }: { children: ReactNode }) {
-  const auth = useOptionalAuth();
-  // Same owner token as the theme caches, from the same function, so widening
+  // Same owner token as the theme caches, from the same hook, so widening
   // ownership can never move one provider and leave this one behind.
-  const cacheOwner = appearanceCacheOwner({
-    loading: auth?.loading ?? false,
-    user: auth?.user ? { id: auth.user.id } : null,
-  });
+  const cacheOwner = useAppearanceCacheOwner();
   const loadApiSettings = cacheOwner !== null;
   const { data: apiSettings } = useSettings({ enabled: loadApiSettings });
   const settingMutation = useSetSetting();

web/src/hooks/useTheme.test.ts

@@ -22,7 +22,11 @@ describe("appearanceCacheOwner", () => {
 
 describe("getInitialTheme", () => {
   beforeEach(() => {
-    Object.values(storage.KEYS).forEach((key) => storage.remove(key));
+    // Not storage.remove over storage.KEYS: appearanceCache writes namespaced
+    // keys ("silo-theme:1") and an owner pointer, none of which appear in
+    // storage.KEYS, so that cleanup left both behind and made these cases
+    // order-dependent. storage.test.ts already clears the whole store.
+    localStorage.clear();
   });
 
   it("warms up from the cache while the owner is unknown", () => {

web/src/hooks/useTheme.tsx

@@ -2,16 +2,15 @@ import { createContext, useContext, useEffect, useState, useCallback } from "rea
 import type { ReactNode } from "react";
 import type { ThemeId } from "@/lib/themes";
 import { useSettings, useSetSetting } from "@/hooks/queries/settings";
-import { useOptionalAuth } from "@/hooks/useAuth";
 import { useBranding } from "@/hooks/useBranding";
 import { appearanceCache, storage } from "@/utils/storage";
 import {
-  appearanceCacheOwner,
   getInitialTheme,
   isValidTheme,
   parseHighContrast,
   parseTextScale,
   parseTextWeight,
+  useAppearanceCacheOwner,
 } from "@/hooks/themePreferences";
 import type { TextScale, TextWeight } from "@/hooks/themePreferences";
 
@@ -47,14 +46,10 @@ function applyHighContrastToDOM(value: boolean): void {
 }
 
 export function ThemeProvider({ children }: { children: ReactNode }) {
-  const auth = useOptionalAuth();
   // The account that owns the localStorage warm start. Null while auth is
   // bootstrapping or nobody is signed in, which still trusts the cache so the
   // app paints in the last look this device used.
-  const cacheOwner = appearanceCacheOwner({
-    loading: auth?.loading ?? false,
-    user: auth?.user ? { id: auth.user.id } : null,
-  });
+  const cacheOwner = useAppearanceCacheOwner();
   const loadApiTheme = cacheOwner !== null;
 
   const [themePreference, setThemePreference] = useState<ThemeId>(() =>