fix(settings): reduce settings desync — server-sourced defaults + atomic save - #519
Open
pnewsam wants to merge 2 commits into
Open
fix(settings): reduce settings desync — server-sourced defaults + atomic save#519pnewsam wants to merge 2 commits into
pnewsam wants to merge 2 commits into
Conversation
…the server (ENG-1125)
Client half of the ENG-941 settings-CRUD refactor (PR1). App.jsx seeded its
settings state from a literal object of defaults, a second copy of a contract
the server already owns. It had drifted: showDots was False here while the
server default is True (the server value already won post-fetch, so this only
ever produced a wrong-value flash on first paint), and defaultModel hard-coded
a model name.
The server (GET /settings/) is the single source of truth and returns every
field's resolved default, so the client doesn't need its own. This removes the
literal and seeds first paint from a read-through cache of the last successful
fetch instead:
- Add lib/settingsCache.{js,test.js}: load/persist the last settings blob in
localStorage. A cache of the server, never a competing default — written only
by fetchSettings, read only to seed useState before the boot fetch lands.
- App.jsx: useState(loadCachedSettings) in place of the hard-coded object.
- api.js: fetchSettings caches its result on success.
On the very first launch the cache is empty and the app renders the server's
values within one fetch (getAgentLabel and the accent/dots reads are all
null-safe on a bare object). No new endpoint, no storage or auth change.
Verified: typecheck, cowork purity, and the full unit suite (603) pass.
Needs an app first-paint smoke in QA (cold start with an empty cache).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…loop (ENG-1126)
Client half of the ENG-941 settings-CRUD refactor (PR2). updateSettings sent one
PUT /settings/{key} per changed key, so a failure partway through left the DB
half-saved (earlier keys committed, later ones not) and threw with a partial
`updated` list. It now sends a single PUT /settings with all changed keys, which
the server applies in one transaction — all-or-nothing.
Pairs with cowork-server #248 (the bulk endpoint). No storage or auth change.
Verified: typecheck, purity, full unit suite pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
pnewsam
marked this pull request as ready for review
July 29, 2026 00:27
pnewsam
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Saving user settings has been a source of many bugs. We've applied many patches but there are some root causes that have gone unaddressed, as described in ENG-941. This makes a first pass at improving some of the settings code design.
This PR is paired with the backend PR. It includes two changes:
App.jsxseeded settings from a hard-coded object that had drifted from the server. This removes that object and replaces it with a fetch of the server'sGET /settings/, which already returns defaults.updateSettingssent onePUT /settings/{key}per changed key so any failure resulted in a partial write. It now sends a single all-or-nothing update request to a new bulk update endpoint.References