Providers redesign, LLM API-key field, and gate device sync out of the binary

[joestump-agent]

Summary

Three related pieces, reworked onto current main (Providers is now a Settings tab; the LLM tab with live-apply already exists upstream):

1. LLM API-key field on the Settings → LLM tab.
2. Providers page redesign (2×2 grid, Telegram "coming soon", auto-refresh, last-synced).
3. Device sync gated out of the release binary behind a build tag.

LLM API key (Settings → LLM tab)

• Adds a write-only API-key field. The value is never rendered back (the form shows only whether a key is set); a blank submission keeps the current key; a new value replaces it. Applies live like the other LLM settings (swaps the running client, no restart).
• Persisted to the 0600 config file via config.SaveLLM (llm.api_key), threaded through llm.Settings + the Applier. MSGBROWSE_LLM_API_KEY still overrides at startup. (Option A: config-file storage — chosen deliberately so a desktop user without a handy env var can set a key.)

Providers page (the /providers Settings tab)

• Source cards laid out as a 2×2 grid (1 column on narrow viewports) plus a Telegram "coming soon" placeholder card (dimmed, non-interactive, not wired into internal/source).
• Removed the one-shot "Refresh all sources" button + /setup/refresh-all route; replaced with a background auto-refresh scheduler (providers.refresh_interval, default 6h, 0 disables) that reuses the per-source concurrency guard, so it can't collide with a manual Refresh.
• "Last synced" timestamp per Enabled/Synced card, from a new store.LastSyncTimes() over ingest_runs.

Device sync gated out of the binary (ADR-0021 / SPEC-0014)

• Behind the devicesync build tag. The default msgbrowse binary links with zero internal/syncthing symbols and no engine constructors (verified with go tool nm); build with -tags devicesync to include it.
• serve + desktop wiring, the msgbrowse devices namespace, and doctor's device checks are split into tagged real / untagged stub files behind a wireDeviceSync seam.
• The Device sync UI on /settings and /status is hidden via a compile-time flag (web.SetDeviceSyncFeature), so a release build renders no dead surface. New tests assert the feature-off hiding.

Verification

go build ./... and go test ./... pass for both build variants (default and -tags devicesync), across the main module and the desktop module.

Note: the built internal/web/static/app.css was hand-mirrored from internal/web/tailwind/input.css because the Tailwind CLI is unreachable in this environment (proxy 403); run make css to regenerate it cleanly when online.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PruXCwhbcG7C6U88x7wykZ

[joestump]

Each of the three pieces is competently executed in isolation — write-only key field with a set/keep indicator, the Providers grid + scheduler reusing refreshEnabledSources under the Runner's per-source guard, build-tag stubs with feature-gated tests, and everything builds/vets/tests clean under all tag combinations ("", devicesync, devicesync desktop). But this PR bundles several load-bearing problems that need fixing before it lands. All of the below were verified against the branch, not just eyeballed.

Blocking

1. An env-only API key gets silently persisted to disk. A blank key field resolves to s.currentLLM().APIKey (llmsettings.go, keptKey path) — and at boot viper maps MSGBROWSE_LLM_API_KEY → llm.api_key (config.go), so the live key may be the env secret. Any unrelated save (say, editing the base URL) then passes it to config.SaveLLM, which now always writes llm.api_key. A user who deliberately kept the key out of files — the previously documented posture — has it copied into config.yaml, where it survives backups and dotfile syncs, and unsetting the env var no longer removes it. The old save.go guaranteed exactly the opposite ("a pre-existing llm.api_key … this code never writes"). Suggested fix: only persist a key the user explicitly typed into the form; a kept key should keep the file's value, not the resolved live value.

2. A stored key can never be cleared from the UI — and it follows you to new endpoints. Blank-means-keep leaves no removal affordance, directly contradicting SaveLLM's doc ("An empty apiKey is written as an empty value (clearing any previous key)"). Worse: repoint Base URL at a different provider/local proxy and the retained key is sent as the Authorization bearer to the new host — a credential leak to an unintended party. Needs an explicit "clear key" control (checkbox or button), and arguably a nudge when base_url changes while a key is being kept.

3. ADR-0009 / ADR-0010 still say the opposite. ADR-0009 decision 6 ("Secrets via env only", with "Secrets in the config file" listed as rejected) and ADR-0010 decision 6 ("The LLM API key is env-only … never baked into the image or a committed file") are both still Accepted and unmodified, and this PR also deletes the test that pinned "SaveLLM wrote an api_key — it must never do that". If key-in-0600-config is the product choice (it's defensible for a desktop app), amend/supersede both ADRs in this same PR so docs/adr keeps describing reality.

4. device_sync.enabled: true is silently ignored in the shipping build. The untagged wireDeviceSync stub returns nil without even a log line (serve_nodevicesync.go) — on main, serve with sync enabled either actually replicated or failed fast on an unstartable engine. And doctor_devices_stub.go reports statusPass unconditionally. Since every default artifact is untagged (make build, Dockerfile, and DESKTOP_TAGS all omit devicesync), an operator with paired replicas who upgrades gets: serve starts green, replication silently stops, doctor says PASS. Minimum fix: both stubs should check cfg.DeviceSync.Enabled and log/report a prominent warning that the configured feature is not in this binary.

5. The build tag doesn't deliver what the comments claim. internal/web still unconditionally imports internal/devsync (statusData/SetSyncMonitor/logs feed), which pulls internal/syncthing — go list -deps ./cmd/msgbrowse on the untagged tree lists both packages. The stubs' "links WITHOUT internal/devsync or internal/syncthing" claim is false today: only the wiring and UI are gated; the not-release-ready code still ships in the binary. Either sever the dependency at the web boundary (web-local status types or a tagged web file) or correct the comments and the PR's "gated out of the binary" claim.

6. "Last synced" vanishes on every card fragment swap. Only setupCards() backfills LastSynced/HasLastSynced (setup.go); the fragments rendered by the enable/refresh status poller (enable.go), /setup/recheck (recheck.go), and the disable flow (disable.go) call setupCardFor directly, so the stamp disappears at the exact moment a refresh completes and only returns on full page reload. Move the backfill into setupCardFor (or fetch LastSyncTimes there).

7. Hand-edited generated app.css. Same as #19: make css on this branch produces a diff, so the css.yml drift guard fails; and this PR's appended tail conflicts with #19's on the same minified line. Regenerate and commit; never hand-resolve that conflict.

Should-fix

8. "Last synced" advances on all-failed runs. LastSyncTimes is MAX(finished_at) with no errors filter, and per-conversation import failures still record the run row (run.Errors++ then unconditional RecordIngestRun). A broken/unparseable archive shows "Last synced just now" forever while importing nothing. (Scoped: a total exporter failure records no row and surfaces as a failed job — this is specifically the partial/parse-failure class.) Consider WHERE errors = 0 or exposing the error count on the card.

9. Issue joestump#162 footer regression. The deliberately three-way footer collapsed to two-way: with nothing detected and nothing enabled (fresh machine), /providers now claims "Enabled sources refresh automatically … use a source's Refresh icon" while every card reads Not detected and renders no Refresh control — the same footer-contradicts-cards class joestump#162 fixed. Restore the empty-state branch; also consider the copy when refresh_interval: 0 disables auto-refresh.

10. Docs not updated for the gating. README §Device sync and docs-site features/device-sync.md still walk users through device_sync.enabled, Settings → Device sync, and msgbrowse devices … — all unreachable in a default build; only config.example.yaml was touched, and the devicesync build tag is documented nowhere user-facing.

11. autocomplete="off" on the key field. Chrome/Safari ignore off on type=password; use autocomplete="new-password". Otherwise a password manager can autofill a login password that — because the field never echoes and non-blank replaces — silently becomes the API key on an unrelated save.

Non-blocking cleanups

• LastSyncTimes inlines time.Parse(time.RFC3339, …); parseRFC3339 (used by LatestIngestRun on the same column) has different failure semantics — use one path.
• validateLLMAPIKey is a byte-for-byte clone of validateLLMModel modulo the length constant — share a validatePrintable(s, max).
• The "Last synced" stamp is the third literal copy of the "2006-01-02 15:04" local-format (settings.go has PairedAt/LastSeen) — worth one helper.
• .setup-card-coming-soon/.setup-badge-coming-soon duplicate the not-detected pair's values; and setup_coming_soon_card takes the source name as a parameter while hardcoding Telegram's tint/icon — either parameterize fully or hardcode honestly.
• Each 6h tick re-runs the full export per enabled source even when nothing changed (only the import is incremental) — a cheap staleness pre-check (source DB mtime vs last finished_at) would spare large archives minutes of subprocess churn 4×/day. Relatedly, the scheduler makes ingest_runs grow unboundedly (~1.5k rows/source/year) and MAX(finished_at) has no (source, finished_at) index.

For the record, one suspicion was checked and refuted: the auto-refresh goroutine's lifecycle is handled correctly — both call sites create the ctx via signal.NotifyContext with defer stop(), no tick can fire in the failure window, and a closed store degrades to logged warnings.

🤖 Posted on behalf of @joestump by Claude.