refactor(server): one runtime-settings overlay module

[tarakanof]

Closes #144

Design summary: see the spec comment on #144.

What

One module, cmd/ember/settings_overlay.go, owns merge, validation, atomic swap, persistence and re-apply for every runtime-editable setting. Each feature registers a settingSpec[D]:

type settingSpec[D any] struct {
    key   string                 // store key
    view  func(Config) D         // effective → DTO (GET, merge seed, persisted blob)
    apply func(*Config, D) error // DTO → config copy; validation here
    after func(Config)           // optional side effects
}
func register[D any](o *settingsOverlay, spec settingSpec[D]) *setting[D]
func (s *setting[D]) get() D
func (s *setting[D]) put(patch []byte) (D, error)
func (o *settingsOverlay) reapply()

Registered: pomodoro, weather, meetings, usage, display, quiet. The clock URL stays outside: it's a raw string, discovery swaps it in memory, and a reload re-applies it only when the file URL changed. Hidden apps are a set toggle.

Before / after

before                                          after
display_config.go  DTO→updateConfig→Put/Get     display_config.go   settingSpec (view/apply)
quiet_config.go    DTO→updateConfig→Put/Get     quiet_config.go     settingSpec
usage_config.go    DTO→clamp→updateConfig→...   usage_config.go     settingSpec
weather.go         apply/loadPersisted (copy)   weather.go          settingSpec + after
meetings.go        apply/loadPersisted (copy)   meetings.go         settingSpec + after
pomodoro_http.go   own cfgMu merge + load       pomodoro_http.go    settingSpec + after
main.go   initPomodoro, initWeather,            main.go   initPomodoro; settings.reapply()
          initMeetings, 3× loadPersisted
admin.go  resync+load, 2× load, 3× load         admin.go  resync engine; settings.reapply()
                                                settings_overlay.go  merge / validate+swap+persist
                                                                     under cfgMu / reapply loop

Behaviour changes

1. Bug fix (regression test first): a partial PUT /v1/display/config (e.g. {"attention_chime":false}) merged into a zero DTO and failed the hold-seconds check with a 400. It now changes only the named field. Stored display blobs load the same way.

2. A partial weather or meetings PUT now merges. Before, omitted fields were reset to zero or their defaults.

3. usage_threshold_pct outside 0..100 now returns a 400. Before, it was clamped silently. The menu's stepper range is already 0...100. A stored usage_json blob with an out-of-range threshold (only a hand-edited DB can hold one) is now ignored whole, so all four fields fall back to the baseline. Before, it was clamped and its toggles still applied.

4. A stored blob that is missing a newer key keeps the current (baseline) value. Quiet and usage already did this; now every setting does.

5. A settings PUT body must be a JSON object: null, arrays and strings get a 400. A wrong-typed value is a 400 through rejectBody, so the "request rejected" log line stays.

6. The stored blob is written under cfgMu, so racing PUTs can't leave an older value in the store than the one that's live.

7. Pomodoro: the after hook stops a running timer whenever the merged config is disabled, not only when the body named enabled:false. Normally the two are the same.

8. Startup: the separate "weather/meetings store init failed" warnings are gone, since those functions only reopened the same store initPomodoro opens. The pomodoro-init warning now says settings won't persist. reapply runs before the "pomodoro wired" log, so that line reports the effective enabled.

9. weather.forecast_hours (review follow-up, flagged by refactor(server): one tile module for push, adopt and preview #145/refactor(server): one tile module for push, adopt and preview #156): the clock's forecastWindow draws any 1..24, but file load raised 1..5 to 6. It now keeps 1..24 (0/absent or >24 → 24). A settings PUT outside 0..24 is a 400 (before, there was no check), and 0 means the full day. The docs now say 1..24.

Shared-file hunks

• main.go: startup block, which runs initPomodoro → settings.reapply(), with the init/load calls removed.
• app.go: settings field, newAppSettings in NewApp, and tryUpdateConfig (updateConfig delegates to it).
• admin.go: the reload re-apply list becomes settings.reapply().
• device.go: comment only.
• coordinator_weather_test.go: one assertion updated (3 hours is kept, not raised to 6).

Tests

• settings_overlay_test.go tests the module through its interface with an in-memory KV adapter: merge, whole-map replace without aliasing, invalid input changes nothing and persists nothing, case-insensitive keys, reapply from a legacy blob, invalid or missing blobs, and no store.
• settings_merge_test.go covers partial weather and meetings PUTs and non-object bodies on all five handlers. display_config_test.go covers the partial display PUT.
• Existing tests that called applyXSettings / loadPersistedX now go through settings.X.put / settings.reapply().
• Go 1.26 gofmt -l is clean. go vet and go test -race pass on every package except ember-claude-producer, which is excluded until fix: keep producer LaunchAgents registered and running (#142) #143. No Swift changes.

[tarakanof]

Review: #155 (settings overlay, closes #144)

Verdict: merge as-is. No blockers or should-fix items. There are four nits, and none of them blocks the merge.

What I checked

• Differential test, main vs branch. I wrote a throwaway test and ran the same file on both trees. For all six endpoints (pomodoro, weather, meetings, usage, display, quiet) it covered:

  • GET before and after a full PUT
  • a full PUT with the body the Swift ServerConfigModel sends, i.e. every key including ics_urls_configured and icon_ids:{}
  • the stored blob after the PUT
  • re-apply of 0.28-style blobs that have a missing key and an unknown key

  Output was byte-identical on both trees. The only difference was a hand-made usage blob with usage_threshold_pct:150 (see nit 1).

• GET shapes. Unchanged. view is the old xDTO() or cfg.Load().X, moved without changes.

• After hooks. For each setting, the side effects and their order match the old appliers. Examples: weather is nudgePomo then go ensureNativeIcons; pomodoro is engine update, stop, nudge, icons. They run once per successful put, don't run on a 400, and run outside cfgMu. The old code had no weather refetch or meetings poll restart, and the new code has none either.

• /admin/reload. reapply() runs after cfgMu.Unlock(). Pomodoro, weather, meetings, usage, display and quiet keep their relative order. loadPersistedDeviceBaseURL now runs before the six instead of after meetings. That is harmless, arguably better: the weather icon goroutine now sees the final clock URL.

• Startup. The old code ran initWeather/initMeetings, which only re-opened the same Pomodoro.DBPath store. loadHiddenApps is still called by initPomodoro, and a store-open failure fails the same way either way.

• Concurrency. tryUpdateConfig holds cfgMu across mutate, which now includes PutSetting, a SQLite write. The store has no Go mutex; it uses SetMaxOpenConns(1) and no transactions. No path takes the DB connection and then cfgMu, view/apply never re-enter cfgMu, and the after hooks run outside the lock. I found no deadlock. One cost: config writers such as discovery and reload can now wait on a slow DB query. That is acceptable for this workload.

• Usage clamp → 400. The Settings stepper is 0...100. Every stored usage_json blob since the threshold was added (f61a264) holds a clamped value, and the seed view clamps too. So a server-written blob can't hit the new 400.

• Tests. go test -race -count=2 passes on every package except ember-claude-producer, which I excluded because the branch predates #143. CI is green. The PR merges cleanly into current main.

• Design. The module is deep: six callers supply key/view/apply/after, and merge, atomic swap, persist and re-apply sit behind one seam. settingsKV has two real adapters, the store and the test mapKV. The deletion test passes: removing the module would spread the merge and persist logic back into six copies. I agree with keeping device URL and hidden apps out.

Nits

1. A stored usage blob out of range now drops the whole override. A stored usage_json with usage_threshold_pct outside 0..100 used to be clamped, and its toggles still applied. Now the whole override is ignored and all four fields fall back to the baseline. Only a hand-edited DB can produce such a blob, but it's worth one line under behaviour change 3.
2. Type errors skip the "request rejected" log line. A body with a wrong type (e.g. "focus_minutes":"x") now fails in mergeSetting and goes out through writeError 400. It no longer goes through rejectBody, so it doesn't log that line. The status is unchanged.
3. The appSettings comment points at the wrong place. It says "The field order below is the reapply order". The order that matters is the element order of the composite literal in newAppSettings. The two match today, but the comment points at the wrong thing to keep in sync.
4. Behaviour change 7 has no test. Pomodoro now stops a running timer whenever the merged config is disabled. A small test would pin it: PUT {"enabled":false} while running stops the timer, and a PUT that omits enabled while enabled does not.